From: Greg Kurz <groug@kaod.org> To: Miklos Szeredi <miklos@szeredi.hu> Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Vivek Goyal <vgoyal@redhat.com>, virtio-fs@redhat.com, Greg Kurz <groug@kaod.org> Subject: [PATCH 3/4] fuse: Call vfs_get_tree() for submounts Date: Tue, 25 May 2021 17:02:29 +0200 [thread overview] Message-ID: <20210525150230.157586-4-groug@kaod.org> (raw) In-Reply-To: <20210525150230.157586-1-groug@kaod.org> We recently fixed an infinite loop by setting the SB_BORN flag on submounts along with the write barrier needed by super_cache_count(). This is the job of vfs_get_tree() and FUSE shouldn't have to care about the barrier at all. Split out some code from fuse_dentry_automount() to a new dedicated fuse_get_tree_submount() handler for submounts and call vfs_get_tree(). The fs_private field of the filesystem context isn't used with submounts : hijack it to pass the FUSE inode of the mount point down to fuse_get_tree_submount(). Finally, adapt virtiofs to use this. Signed-off-by: Greg Kurz <groug@kaod.org> --- fs/fuse/dir.c | 58 +++++++-------------------------------------- fs/fuse/fuse_i.h | 6 +++++ fs/fuse/inode.c | 44 ++++++++++++++++++++++++++++++++++ fs/fuse/virtio_fs.c | 3 +++ 4 files changed, 62 insertions(+), 49 deletions(-) diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 3b0482738741..97649dcfeccd 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -309,12 +309,8 @@ static int fuse_dentry_delete(const struct dentry *dentry) static struct vfsmount *fuse_dentry_automount(struct path *path) { struct fs_context *fsc; - struct fuse_mount *parent_fm = get_fuse_mount_super(path->mnt->mnt_sb); - struct fuse_conn *fc = parent_fm->fc; - struct fuse_mount *fm; struct vfsmount *mnt; struct fuse_inode *mp_fi = get_fuse_inode(d_inode(path->dentry)); - struct super_block *sb; int err; fsc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry); @@ -323,47 +319,17 @@ static struct vfsmount *fuse_dentry_automount(struct path *path) goto out; } - err = -ENOMEM; - fm = kzalloc(sizeof(struct fuse_mount), GFP_KERNEL); - if (!fm) - goto out_put_fsc; - - fsc->s_fs_info = fm; - sb = sget_fc(fsc, NULL, set_anon_super_fc); - if (IS_ERR(sb)) { - err = PTR_ERR(sb); - kfree(fm); - goto out_put_fsc; - } - fm->fc = fuse_conn_get(fc); - - /* Initialize superblock, making @mp_fi its root */ - err = fuse_fill_super_submount(sb, mp_fi); - if (err) { - fuse_conn_put(fc); - kfree(fm); - sb->s_fs_info = NULL; - goto out_put_sb; - } - /* - * FIXME: setting SB_BORN requires a write barrier for - * super_cache_count(). We should actually come - * up with a proper ->get_tree() implementation - * for submounts and call vfs_get_tree() to take - * care of the write barrier. + * Hijack fsc->fs_private to pass the mount point inode to + * fuse_get_tree_submount(). It *must* be NULLified afterwards + * to avoid the inode pointer to be passed to kfree() when + * the context gets freed. */ - smp_wmb(); - sb->s_flags |= SB_BORN; - - sb->s_flags |= SB_ACTIVE; - fsc->root = dget(sb->s_root); - /* We are done configuring the superblock, so unlock it */ - up_write(&sb->s_umount); - - down_write(&fc->killsb); - list_add_tail(&fm->fc_entry, &fc->mounts); - up_write(&fc->killsb); + fsc->fs_private = mp_fi; + err = vfs_get_tree(fsc); + fsc->fs_private = NULL; + if (err) + goto out_put_fsc; /* Create the submount */ mnt = vfs_create_mount(fsc); @@ -375,12 +341,6 @@ static struct vfsmount *fuse_dentry_automount(struct path *path) put_fs_context(fsc); return mnt; -out_put_sb: - /* - * Only jump here when fsc->root is NULL and sb is still locked - * (otherwise put_fs_context() will put the superblock) - */ - deactivate_locked_super(sb); out_put_fsc: put_fs_context(fsc); out: diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 7e463e220053..d7fcf59a6a0e 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -1090,6 +1090,12 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx); int fuse_fill_super_submount(struct super_block *sb, struct fuse_inode *parent_fi); +/* + * Get the mountable root for the submount + * @fsc: superblock configuration context + */ +int fuse_get_tree_submount(struct fs_context *fsc); + /* * Remove the mount from the connection * diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index 393e36b74dc4..433ca2b13046 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -1313,6 +1313,50 @@ int fuse_fill_super_submount(struct super_block *sb, return 0; } +/* Filesystem context private data holds the FUSE inode of the mount point */ +int fuse_get_tree_submount(struct fs_context *fsc) +{ + struct fuse_mount *fm; + struct fuse_inode *mp_fi = fsc->fs_private; + struct fuse_conn *fc = get_fuse_conn(&mp_fi->inode); + struct super_block *sb; + int err; + + fm = kzalloc(sizeof(struct fuse_mount), GFP_KERNEL); + if (!fm) + return -ENOMEM; + + fsc->s_fs_info = fm; + sb = sget_fc(fsc, NULL, set_anon_super_fc); + if (IS_ERR(sb)) { + kfree(fm); + return PTR_ERR(sb); + } + fm->fc = fuse_conn_get(fc); + + /* Initialize superblock, making @mp_fi its root */ + err = fuse_fill_super_submount(sb, mp_fi); + if (err) { + fuse_conn_put(fc); + kfree(fm); + sb->s_fs_info = NULL; + deactivate_locked_super(sb); + return err; + } + + sb->s_flags |= SB_ACTIVE; + fsc->root = dget(sb->s_root); + /* We are done configuring the superblock, so unlock it */ + up_write(&sb->s_umount); + + down_write(&fc->killsb); + list_add_tail(&fm->fc_entry, &fc->mounts); + up_write(&fc->killsb); + + return 0; +} +EXPORT_SYMBOL_GPL(fuse_get_tree_submount); + int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) { struct fuse_dev *fud = NULL; diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index bcb8a02e2d8b..e12e5190352c 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -1420,6 +1420,9 @@ static int virtio_fs_get_tree(struct fs_context *fsc) unsigned int virtqueue_size; int err = -EIO; + if (fsc->purpose == FS_CONTEXT_FOR_SUBMOUNT) + return fuse_get_tree_submount(fsc); + /* This gets a reference on virtio_fs object. This ptr gets installed * in fc->iq->priv. Once fuse_conn is going away, it calls ->put() * to drop the reference to this object. -- 2.31.1
WARNING: multiple messages have this Message-ID (diff)
From: Greg Kurz <groug@kaod.org> To: Miklos Szeredi <miklos@szeredi.hu> Cc: linux-fsdevel@vger.kernel.org, virtio-fs@redhat.com, linux-kernel@vger.kernel.org, Vivek Goyal <vgoyal@redhat.com> Subject: [Virtio-fs] [PATCH 3/4] fuse: Call vfs_get_tree() for submounts Date: Tue, 25 May 2021 17:02:29 +0200 [thread overview] Message-ID: <20210525150230.157586-4-groug@kaod.org> (raw) In-Reply-To: <20210525150230.157586-1-groug@kaod.org> We recently fixed an infinite loop by setting the SB_BORN flag on submounts along with the write barrier needed by super_cache_count(). This is the job of vfs_get_tree() and FUSE shouldn't have to care about the barrier at all. Split out some code from fuse_dentry_automount() to a new dedicated fuse_get_tree_submount() handler for submounts and call vfs_get_tree(). The fs_private field of the filesystem context isn't used with submounts : hijack it to pass the FUSE inode of the mount point down to fuse_get_tree_submount(). Finally, adapt virtiofs to use this. Signed-off-by: Greg Kurz <groug@kaod.org> --- fs/fuse/dir.c | 58 +++++++-------------------------------------- fs/fuse/fuse_i.h | 6 +++++ fs/fuse/inode.c | 44 ++++++++++++++++++++++++++++++++++ fs/fuse/virtio_fs.c | 3 +++ 4 files changed, 62 insertions(+), 49 deletions(-) diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 3b0482738741..97649dcfeccd 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -309,12 +309,8 @@ static int fuse_dentry_delete(const struct dentry *dentry) static struct vfsmount *fuse_dentry_automount(struct path *path) { struct fs_context *fsc; - struct fuse_mount *parent_fm = get_fuse_mount_super(path->mnt->mnt_sb); - struct fuse_conn *fc = parent_fm->fc; - struct fuse_mount *fm; struct vfsmount *mnt; struct fuse_inode *mp_fi = get_fuse_inode(d_inode(path->dentry)); - struct super_block *sb; int err; fsc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry); @@ -323,47 +319,17 @@ static struct vfsmount *fuse_dentry_automount(struct path *path) goto out; } - err = -ENOMEM; - fm = kzalloc(sizeof(struct fuse_mount), GFP_KERNEL); - if (!fm) - goto out_put_fsc; - - fsc->s_fs_info = fm; - sb = sget_fc(fsc, NULL, set_anon_super_fc); - if (IS_ERR(sb)) { - err = PTR_ERR(sb); - kfree(fm); - goto out_put_fsc; - } - fm->fc = fuse_conn_get(fc); - - /* Initialize superblock, making @mp_fi its root */ - err = fuse_fill_super_submount(sb, mp_fi); - if (err) { - fuse_conn_put(fc); - kfree(fm); - sb->s_fs_info = NULL; - goto out_put_sb; - } - /* - * FIXME: setting SB_BORN requires a write barrier for - * super_cache_count(). We should actually come - * up with a proper ->get_tree() implementation - * for submounts and call vfs_get_tree() to take - * care of the write barrier. + * Hijack fsc->fs_private to pass the mount point inode to + * fuse_get_tree_submount(). It *must* be NULLified afterwards + * to avoid the inode pointer to be passed to kfree() when + * the context gets freed. */ - smp_wmb(); - sb->s_flags |= SB_BORN; - - sb->s_flags |= SB_ACTIVE; - fsc->root = dget(sb->s_root); - /* We are done configuring the superblock, so unlock it */ - up_write(&sb->s_umount); - - down_write(&fc->killsb); - list_add_tail(&fm->fc_entry, &fc->mounts); - up_write(&fc->killsb); + fsc->fs_private = mp_fi; + err = vfs_get_tree(fsc); + fsc->fs_private = NULL; + if (err) + goto out_put_fsc; /* Create the submount */ mnt = vfs_create_mount(fsc); @@ -375,12 +341,6 @@ static struct vfsmount *fuse_dentry_automount(struct path *path) put_fs_context(fsc); return mnt; -out_put_sb: - /* - * Only jump here when fsc->root is NULL and sb is still locked - * (otherwise put_fs_context() will put the superblock) - */ - deactivate_locked_super(sb); out_put_fsc: put_fs_context(fsc); out: diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 7e463e220053..d7fcf59a6a0e 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -1090,6 +1090,12 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx); int fuse_fill_super_submount(struct super_block *sb, struct fuse_inode *parent_fi); +/* + * Get the mountable root for the submount + * @fsc: superblock configuration context + */ +int fuse_get_tree_submount(struct fs_context *fsc); + /* * Remove the mount from the connection * diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index 393e36b74dc4..433ca2b13046 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -1313,6 +1313,50 @@ int fuse_fill_super_submount(struct super_block *sb, return 0; } +/* Filesystem context private data holds the FUSE inode of the mount point */ +int fuse_get_tree_submount(struct fs_context *fsc) +{ + struct fuse_mount *fm; + struct fuse_inode *mp_fi = fsc->fs_private; + struct fuse_conn *fc = get_fuse_conn(&mp_fi->inode); + struct super_block *sb; + int err; + + fm = kzalloc(sizeof(struct fuse_mount), GFP_KERNEL); + if (!fm) + return -ENOMEM; + + fsc->s_fs_info = fm; + sb = sget_fc(fsc, NULL, set_anon_super_fc); + if (IS_ERR(sb)) { + kfree(fm); + return PTR_ERR(sb); + } + fm->fc = fuse_conn_get(fc); + + /* Initialize superblock, making @mp_fi its root */ + err = fuse_fill_super_submount(sb, mp_fi); + if (err) { + fuse_conn_put(fc); + kfree(fm); + sb->s_fs_info = NULL; + deactivate_locked_super(sb); + return err; + } + + sb->s_flags |= SB_ACTIVE; + fsc->root = dget(sb->s_root); + /* We are done configuring the superblock, so unlock it */ + up_write(&sb->s_umount); + + down_write(&fc->killsb); + list_add_tail(&fm->fc_entry, &fc->mounts); + up_write(&fc->killsb); + + return 0; +} +EXPORT_SYMBOL_GPL(fuse_get_tree_submount); + int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) { struct fuse_dev *fud = NULL; diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index bcb8a02e2d8b..e12e5190352c 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -1420,6 +1420,9 @@ static int virtio_fs_get_tree(struct fs_context *fsc) unsigned int virtqueue_size; int err = -EIO; + if (fsc->purpose == FS_CONTEXT_FOR_SUBMOUNT) + return fuse_get_tree_submount(fsc); + /* This gets a reference on virtio_fs object. This ptr gets installed * in fc->iq->priv. Once fuse_conn is going away, it calls ->put() * to drop the reference to this object. -- 2.31.1
next prev parent reply other threads:[~2021-05-25 15:03 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-25 15:02 [PATCH 0/4] fuse: Some fixes for submounts Greg Kurz 2021-05-25 15:02 ` [Virtio-fs] " Greg Kurz 2021-05-25 15:02 ` [PATCH 1/4] fuse: Fix crash in fuse_dentry_automount() error path Greg Kurz 2021-05-25 15:02 ` [Virtio-fs] " Greg Kurz 2021-05-27 9:51 ` Max Reitz 2021-05-27 9:51 ` Max Reitz 2021-05-25 15:02 ` [PATCH 2/4] fuse: Fix infinite loop in sget_fc() Greg Kurz 2021-05-25 15:02 ` [Virtio-fs] " Greg Kurz 2021-05-27 10:08 ` Max Reitz 2021-05-27 10:08 ` Max Reitz 2021-05-27 12:31 ` Greg Kurz 2021-05-25 15:02 ` Greg Kurz [this message] 2021-05-25 15:02 ` [Virtio-fs] [PATCH 3/4] fuse: Call vfs_get_tree() for submounts Greg Kurz 2021-05-27 13:24 ` Max Reitz 2021-06-03 7:34 ` Greg Kurz 2021-06-03 7:34 ` Greg Kurz 2021-05-25 15:02 ` [PATCH 4/4] fuse: Make fuse_fill_super_submount() static Greg Kurz 2021-05-25 15:02 ` [Virtio-fs] " Greg Kurz 2021-05-27 13:28 ` Max Reitz
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210525150230.157586-4-groug@kaod.org \ --to=groug@kaod.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=miklos@szeredi.hu \ --cc=vgoyal@redhat.com \ --cc=virtio-fs@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.