Allocate struct fuse_dev when opening the device. This means that unlike before, ->private_data is always set to a valid pointer. The use of USE_DEV_SYNC_INIT magic pointer for the private_data is now replaced with a simple bool sync_init member. If sync INIT is not set, I/O on the device returns error before mount. Keep this behavior by checking for the ->fc member. If fud->fc is set, the mount has succeeded. Testing this used READ_ONCE(file->private_data) and smp_mb() to try and provide the necessary semantics. Switch this to smp_store_release() and smp_load_acquire(). Setting fud->fc is protected by fuse_mutex, this is unchanged. Will need this later so the /dev/fuse open file reference is not held during FSCONFIG_CMD_CREATE. Signed-off-by: Miklos Szeredi --- fs/fuse/dev.c | 47 +++++++++++++++++--------------------------- fs/fuse/fuse_dev_i.h | 33 +++++++++++++++++++++++-------- fs/fuse/fuse_i.h | 5 ++--- fs/fuse/inode.c | 35 +++++++++++---------------------- fs/fuse/virtio_fs.c | 2 -- 5 files changed, 56 insertions(+), 66 deletions(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 2c16b94357d5..4795e3d01b75 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -1542,32 +1542,24 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file, static int fuse_dev_open(struct inode *inode, struct file *file) { - /* - * The fuse device's file's private_data is used to hold - * the fuse_conn(ection) when it is mounted, and is used to - * keep track of whether the file has been mounted already. - */ - file->private_data = NULL; + struct fuse_dev *fud = fuse_dev_alloc(); + + if (!fud) + return -ENOMEM; + + file->private_data = fud; return 0; } struct fuse_dev *fuse_get_dev(struct file *file) { - struct fuse_dev *fud = __fuse_get_dev(file); + struct fuse_dev *fud = fuse_file_to_fud(file); int err; - if (likely(fud)) - return fud; - - err = wait_event_interruptible(fuse_dev_waitq, - READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT); + err = wait_event_interruptible(fuse_dev_waitq, fuse_dev_fc_get(fud)); if (err) return ERR_PTR(err); - fud = __fuse_get_dev(file); - if (!fud) - return ERR_PTR(-EPERM); - return fud; } @@ -2534,10 +2526,10 @@ void fuse_wait_aborted(struct fuse_conn *fc) int fuse_dev_release(struct inode *inode, struct file *file) { - struct fuse_dev *fud = __fuse_get_dev(file); + struct fuse_dev *fud = fuse_file_to_fud(file); + struct fuse_conn *fc = fuse_dev_fc_get(fud); - if (fud) { - struct fuse_conn *fc = fud->fc; + if (fc) { struct fuse_pqueue *fpq = &fud->pq; LIST_HEAD(to_end); unsigned int i; @@ -2555,8 +2547,8 @@ int fuse_dev_release(struct inode *inode, struct file *file) WARN_ON(fc->iq.fasync != NULL); fuse_abort_conn(fc); } - fuse_dev_free(fud); } + fuse_dev_free(fud); return 0; } EXPORT_SYMBOL_GPL(fuse_dev_release); @@ -2574,16 +2566,12 @@ static int fuse_dev_fasync(int fd, struct file *file, int on) static int fuse_device_clone(struct fuse_conn *fc, struct file *new) { - struct fuse_dev *fud; + struct fuse_dev *new_fud = fuse_file_to_fud(new); - if (__fuse_get_dev(new)) + if (fuse_dev_fc_get(new_fud)) return -EINVAL; - fud = fuse_dev_alloc_install(fc); - if (!fud) - return -ENOMEM; - - new->private_data = fud; + fuse_dev_install(new_fud, fc); atomic_inc(&fc->dev_count); return 0; @@ -2657,10 +2645,11 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp) static long fuse_dev_ioctl_sync_init(struct file *file) { int err = -EINVAL; + struct fuse_dev *fud = fuse_file_to_fud(file); mutex_lock(&fuse_mutex); - if (!__fuse_get_dev(file)) { - WRITE_ONCE(file->private_data, FUSE_DEV_SYNC_INIT); + if (!fuse_dev_fc_get(fud)) { + fud->sync_init = true; err = 0; } mutex_unlock(&fuse_mutex); diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h index 134bf44aff0d..522b2012cd1f 100644 --- a/fs/fuse/fuse_dev_i.h +++ b/fs/fuse/fuse_dev_i.h @@ -39,18 +39,35 @@ struct fuse_copy_state { } ring; }; -#define FUSE_DEV_SYNC_INIT ((struct fuse_dev *) 1) -#define FUSE_DEV_PTR_MASK (~1UL) +/* + * Lockless access is OK, because fud->fc is set once during mount and is valid + * until the file is released. + */ +static inline struct fuse_conn *fuse_dev_fc_get(struct fuse_dev *fud) +{ + /* Pairs with smp_store_release() in fuse_dev_fc_set() */ + return smp_load_acquire(&fud->fc); +} + +static inline void fuse_dev_fc_set(struct fuse_dev *fud, struct fuse_conn *fc) +{ + /* Pairs with smp_load_acquire() in fuse_dev_fc_get() */ + smp_store_release(&fud->fc, fc); +} + +static inline struct fuse_dev *fuse_file_to_fud(struct file *file) +{ + return file->private_data; +} static inline struct fuse_dev *__fuse_get_dev(struct file *file) { - /* - * Lockless access is OK, because file->private data is set - * once during mount and is valid until the file is released. - */ - struct fuse_dev *fud = READ_ONCE(file->private_data); + struct fuse_dev *fud = fuse_file_to_fud(file); + + if (!fuse_dev_fc_get(fud)) + return NULL; - return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK); + return fud; } struct fuse_dev *fuse_get_dev(struct file *file); diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 7f16049387d1..739220d96b6f 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -576,6 +576,8 @@ struct fuse_pqueue { * Fuse device instance */ struct fuse_dev { + bool sync_init; + /** Fuse connection for this device */ struct fuse_conn *fc; @@ -622,9 +624,6 @@ struct fuse_fs_context { /* DAX device, may be NULL */ struct dax_device *dax_dev; - - /* fuse_dev pointer to fill in, should contain NULL on entry */ - void **fudptr; }; struct fuse_sync_bucket { diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index e57b8af06be9..53663846db36 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -1637,7 +1637,7 @@ EXPORT_SYMBOL_GPL(fuse_dev_alloc); void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc) { - fud->fc = fuse_conn_get(fc); + fuse_dev_fc_set(fud, fuse_conn_get(fc)); spin_lock(&fc->lock); list_add_tail(&fud->entry, &fc->devices); spin_unlock(&fc->lock); @@ -1659,7 +1659,7 @@ EXPORT_SYMBOL_GPL(fuse_dev_alloc_install); void fuse_dev_free(struct fuse_dev *fud) { - struct fuse_conn *fc = fud->fc; + struct fuse_conn *fc = fuse_dev_fc_get(fud); if (fc) { spin_lock(&fc->lock); @@ -1822,7 +1822,7 @@ EXPORT_SYMBOL_GPL(fuse_init_fs_context_submount); int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) { - struct fuse_dev *fud = NULL; + struct fuse_dev *fud = ctx->file ? fuse_file_to_fud(ctx->file) : NULL; struct fuse_mount *fm = get_fuse_mount_super(sb); struct fuse_conn *fc = fm->fc; struct inode *root; @@ -1856,18 +1856,11 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) goto err; } - if (ctx->fudptr) { - err = -ENOMEM; - fud = fuse_dev_alloc_install(fc); - if (!fud) - goto err_free_dax; - } - fc->dev = sb->s_dev; fm->sb = sb; err = fuse_bdi_init(fc, sb); if (err) - goto err_dev_free; + goto err_free_dax; /* Handle umasking inside the fuse code */ if (sb->s_flags & SB_POSIXACL) @@ -1889,15 +1882,15 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) set_default_d_op(sb, &fuse_dentry_operations); root_dentry = d_make_root(root); if (!root_dentry) - goto err_dev_free; + goto err_free_dax; mutex_lock(&fuse_mutex); err = -EINVAL; - if (ctx->fudptr && *ctx->fudptr) { - if (*ctx->fudptr == FUSE_DEV_SYNC_INIT) - fc->sync_init = 1; - else + if (fud) { + if (fuse_dev_fc_get(fud)) goto err_unlock; + if (fud->sync_init) + fc->sync_init = 1; } err = fuse_ctl_add_conn(fc); @@ -1906,8 +1899,8 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) list_add_tail(&fc->entry, &fuse_conn_list); sb->s_root = root_dentry; - if (ctx->fudptr) { - *ctx->fudptr = fud; + if (fud) { + fuse_dev_install(fud, fc); wake_up_all(&fuse_dev_waitq); } mutex_unlock(&fuse_mutex); @@ -1916,9 +1909,6 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) err_unlock: mutex_unlock(&fuse_mutex); dput(root_dentry); - err_dev_free: - if (fud) - fuse_dev_free(fud); err_free_dax: if (IS_ENABLED(CONFIG_FUSE_DAX)) fuse_dax_conn_free(fc); @@ -1944,13 +1934,10 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc) if ((ctx->file->f_op != &fuse_dev_operations) || (ctx->file->f_cred->user_ns != sb->s_user_ns)) return -EINVAL; - ctx->fudptr = &ctx->file->private_data; err = fuse_fill_super_common(sb, ctx); if (err) return err; - /* file->private_data shall be visible on all CPUs after this */ - smp_mb(); fm = get_fuse_mount_super(sb); diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index 2f7485ffac52..f685916754ad 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -1590,8 +1590,6 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc) goto err_free_fuse_devs; } - /* virtiofs allocates and installs its own fuse devices */ - ctx->fudptr = NULL; if (ctx->dax_mode != FUSE_DAX_NEVER) { if (ctx->dax_mode == FUSE_DAX_ALWAYS && !fs->dax_dev) { err = -EINVAL; -- 2.53.0