| Message ID | 20240208184913.484340-4-mathieu.desnoyers@efficios.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce cpu_dcache_is_aliasing() to fix DAX regression | expand |
Mathieu Desnoyers wrote: > In preparation for checking whether the architecture has data cache > aliasing within alloc_dax(), modify the error handling of dm alloc_dev() > to treat alloc_dax() -EOPNOTSUPP failure as non-fatal. > > For the transition, consider that alloc_dax() returning NULL is the > same as returning -EOPNOTSUPP. > > Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches") > Suggested-by: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: Alasdair Kergon <agk@redhat.com> > Cc: Mike Snitzer <snitzer@kernel.org> > Cc: Mikulas Patocka <mpatocka@redhat.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Vishal Verma <vishal.l.verma@intel.com> > Cc: Dave Jiang <dave.jiang@intel.com> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Russell King <linux@armlinux.org.uk> > Cc: linux-arch@vger.kernel.org > Cc: linux-cxl@vger.kernel.org > Cc: linux-fsdevel@vger.kernel.org > Cc: linux-mm@kvack.org > Cc: linux-xfs@vger.kernel.org > Cc: dm-devel@lists.linux.dev > Cc: nvdimm@lists.linux.dev > --- > drivers/md/dm.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/drivers/md/dm.c b/drivers/md/dm.c > index 23c32cd1f1d8..2fc22cae9089 100644 > --- a/drivers/md/dm.c > +++ b/drivers/md/dm.c > @@ -2054,6 +2054,7 @@ static void cleanup_mapped_device(struct mapped_device *md) > static struct mapped_device *alloc_dev(int minor) > { > int r, numa_node_id = dm_get_numa_node(); > + struct dax_device *dax_dev; > struct mapped_device *md; > void *old_md; > > @@ -2122,15 +2123,15 @@ static struct mapped_device *alloc_dev(int minor) > md->disk->private_data = md; > sprintf(md->disk->disk_name, "dm-%d", minor); > > - if (IS_ENABLED(CONFIG_FS_DAX)) { > - md->dax_dev = alloc_dax(md, &dm_dax_ops); > - if (IS_ERR(md->dax_dev)) { > - md->dax_dev = NULL; > + dax_dev = alloc_dax(md, &dm_dax_ops); > + if (IS_ERR_OR_NULL(dax_dev)) { Similar feedback as the pmem change, lets not propagate the mistake that alloc_dax() could return NULL, none of the callers of alloc_dax() properly handled NULL and it was just luck that none of the use cases tried to use alloc_dax() in the CONFIG_DAX=n case.
On 2024-02-08 16:34, Dan Williams wrote: [...] > Similar feedback as the pmem change, lets not propagate the mistake that > alloc_dax() could return NULL, none of the callers of alloc_dax() > properly handled NULL and it was just luck that none of the use cases > tried to use alloc_dax() in the CONFIG_DAX=n case. Done. Thanks, Mathieu
diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 23c32cd1f1d8..2fc22cae9089 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -2054,6 +2054,7 @@ static void cleanup_mapped_device(struct mapped_device *md) static struct mapped_device *alloc_dev(int minor) { int r, numa_node_id = dm_get_numa_node(); + struct dax_device *dax_dev; struct mapped_device *md; void *old_md; @@ -2122,15 +2123,15 @@ static struct mapped_device *alloc_dev(int minor) md->disk->private_data = md; sprintf(md->disk->disk_name, "dm-%d", minor); - if (IS_ENABLED(CONFIG_FS_DAX)) { - md->dax_dev = alloc_dax(md, &dm_dax_ops); - if (IS_ERR(md->dax_dev)) { - md->dax_dev = NULL; + dax_dev = alloc_dax(md, &dm_dax_ops); + if (IS_ERR_OR_NULL(dax_dev)) { + if (IS_ERR(dax_dev) && PTR_ERR(dax_dev) != -EOPNOTSUPP) goto bad; - } - set_dax_nocache(md->dax_dev); - set_dax_nomc(md->dax_dev); - if (dax_add_host(md->dax_dev, md->disk)) + } else { + set_dax_nocache(dax_dev); + set_dax_nomc(dax_dev); + md->dax_dev = dax_dev; + if (dax_add_host(dax_dev, md->disk)) goto bad; }
In preparation for checking whether the architecture has data cache aliasing within alloc_dax(), modify the error handling of dm alloc_dev() to treat alloc_dax() -EOPNOTSUPP failure as non-fatal. For the transition, consider that alloc_dax() returning NULL is the same as returning -EOPNOTSUPP. Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing caches") Suggested-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Alasdair Kergon <agk@redhat.com> Cc: Mike Snitzer <snitzer@kernel.org> Cc: Mikulas Patocka <mpatocka@redhat.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Vishal Verma <vishal.l.verma@intel.com> Cc: Dave Jiang <dave.jiang@intel.com> Cc: Matthew Wilcox <willy@infradead.org> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Russell King <linux@armlinux.org.uk> Cc: linux-arch@vger.kernel.org Cc: linux-cxl@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org Cc: linux-mm@kvack.org Cc: linux-xfs@vger.kernel.org Cc: dm-devel@lists.linux.dev Cc: nvdimm@lists.linux.dev --- drivers/md/dm.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)