Mathieu Desnoyers wrote: > In preparation for checking whether the architecture has data cache > aliasing within alloc_dax(), modify the error handling of virtio > virtio_fs_setup_dax() to treat alloc_dax() -EOPNOTSUPP failure as > non-fatal. > > Co-developed-by: Dan Williams <[email protected]> > Signed-off-by: Dan Williams <[email protected]> > Fixes: d92576f1167c ("dax: does not work correctly with virtual aliasing > caches") > Signed-off-by: Mathieu Desnoyers <[email protected]> > Cc: Alasdair Kergon <[email protected]> > Cc: Mike Snitzer <[email protected]> > Cc: Mikulas Patocka <[email protected]> > Cc: Andrew Morton <[email protected]> > Cc: Linus Torvalds <[email protected]> > Cc: Dan Williams <[email protected]> > Cc: Vishal Verma <[email protected]> > Cc: Dave Jiang <[email protected]> > Cc: Matthew Wilcox <[email protected]> > Cc: Arnd Bergmann <[email protected]> > Cc: Russell King <[email protected]> > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > --- > fs/fuse/virtio_fs.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c > index 5f1be1da92ce..f9acd9972af2 100644 > --- a/fs/fuse/virtio_fs.c > +++ b/fs/fuse/virtio_fs.c > @@ -16,6 +16,7 @@ > #include <linux/fs_context.h> > #include <linux/fs_parser.h> > #include <linux/highmem.h> > +#include <linux/cleanup.h> > #include <linux/uio.h> > #include "fuse_i.h" > > @@ -795,8 +796,11 @@ static void virtio_fs_cleanup_dax(void *data) > put_dax(dax_dev); > } > > +DEFINE_FREE(cleanup_dax, struct dax_dev *, if (!IS_ERR(_T)) > virtio_fs_cleanup_dax(_T)) > + > static int virtio_fs_setup_dax(struct virtio_device *vdev, struct virtio_fs > *fs) > { > + struct dax_device *dax_dev __free(cleanup_dax) = ERR_PTR(-EOPNOTSUPP); > struct virtio_shm_region cache_reg; > struct dev_pagemap *pgmap; > bool have_cache; > @@ -804,6 +808,12 @@ static int virtio_fs_setup_dax(struct virtio_device > *vdev, struct virtio_fs *fs) > if (!IS_ENABLED(CONFIG_FUSE_DAX)) > return 0; > > + dax_dev = alloc_dax(fs, &virtio_fs_dax_ops); > + if (IS_ERR(dax_dev)) { > + int rc = PTR_ERR(dax_dev); > + return rc == -EOPNOTSUPP ? 0 : rc; > + } > + > /* Get cache region */ > have_cache = virtio_get_shm_region(vdev, &cache_reg, > (u8)VIRTIO_FS_SHMCAP_ID_CACHE); > @@ -849,10 +859,7 @@ static int virtio_fs_setup_dax(struct virtio_device > *vdev, struct virtio_fs *fs) > dev_dbg(&vdev->dev, "%s: window kaddr 0x%px phys_addr 0x%llx len > 0x%llx\n", > __func__, fs->window_kaddr, cache_reg.addr, cache_reg.len); > > - fs->dax_dev = alloc_dax(fs, &virtio_fs_dax_ops); > - if (IS_ERR(fs->dax_dev)) > - return PTR_ERR(fs->dax_dev); > - > + fs->dax_dev = no_free_ptr(dax_dev);
This works because the internals of virtio_fs_cleanup_dax(), "kill_dax() and put_dax()", know how to handle a NULL @dax_dev. It is still early days with the "cleanup" helpers, but I wonder if anyone else cares that the DEFINE_FREE() above does not check for NULL? I think it is ok, but wanted to point that out for the virtiofs folks and wonder what the style should be for things like this going forward. Other growing pains with "cleanup.h" and ERR_PTR is happening over here: http://lore.kernel.org/r/[email protected] ...and that arrived at a similar style as is being used in this patch. In both cases the cleanup function is called on exit with a NULL argument.
