split up ->kill_sb

split up ->kill_sb

[Christoph Hellwig]

Hi Al and Christian,

this series splits ->kill_sb into separate ->shutdown_sb and ->free_sb
methods and then calls generic_shutdown_super from common code to clean
up the file system shutdown interface.

As a first step towards that it moves allocating and freeing the
anonymous block device dev_t into common code. As every super_block must
have a valid s_dev it makes sense to just do that if the file system
didn't set one by itself, and we can also detect if one was assigned
easily when shutting down.

A git tree is available here:

    git://git.infradead.org/users/hch/misc.git fs-kill_sb

Gitweb:

    http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/fs-kill_sb

Diffstat:
 Documentation/filesystems/locking.rst     |    9 -
 Documentation/filesystems/vfs.rst         |   15 ++
 arch/powerpc/platforms/cell/spufs/inode.c |   10 -
 arch/s390/hypfs/inode.c                   |   43 --------
 arch/x86/kernel/cpu/resctrl/rdtgroup.c    |    7 -
 block/bdev.c                              |    1 
 drivers/android/binderfs.c                |   12 --
 drivers/base/devtmpfs.c                   |    8 -
 drivers/dax/super.c                       |    1 
 drivers/dma-buf/dma-buf.c                 |    1 
 drivers/gpu/drm/drm_drv.c                 |    1 
 drivers/infiniband/hw/qib/qib.h           |    4 
 drivers/infiniband/hw/qib/qib_fs.c        |  105 ++++---------------
 drivers/infiniband/hw/qib/qib_init.c      |   32 ++----
 drivers/misc/cxl/api.c                    |    1 
 drivers/misc/ibmasm/ibmasmfs.c            |    8 -
 drivers/mtd/mtdsuper.c                    |   12 --
 drivers/scsi/cxlflash/ocxl_hw.c           |    1 
 drivers/usb/gadget/function/f_fs.c        |    6 -
 drivers/usb/gadget/legacy/inode.c         |   18 +--
 drivers/xen/xenfs/super.c                 |    8 -
 fs/9p/vfs_super.c                         |   16 ---
 fs/adfs/super.c                           |    2 
 fs/affs/super.c                           |    7 -
 fs/afs/super.c                            |   27 ++---
 fs/aio.c                                  |    1 
 fs/anon_inodes.c                          |    1 
 fs/autofs/autofs_i.h                      |    3 
 fs/autofs/init.c                          |    3 
 fs/autofs/inode.c                         |   24 ++--
 fs/befs/linuxvfs.c                        |    2 
 fs/bfs/inode.c                            |    2 
 fs/binfmt_misc.c                          |    8 -
 fs/btrfs/super.c                          |   16 +--
 fs/btrfs/tests/btrfs-tests.c              |    1 
 fs/ceph/super.c                           |   20 +--
 fs/coda/inode.c                           |    1 
 fs/configfs/mount.c                       |    8 -
 fs/cramfs/inode.c                         |    6 -
 fs/debugfs/inode.c                        |    8 -
 fs/devpts/inode.c                         |    6 -
 fs/ecryptfs/main.c                        |   14 --
 fs/efivarfs/super.c                       |   13 +-
 fs/efs/super.c                            |    7 -
 fs/erofs/super.c                          |   25 +---
 fs/exfat/super.c                          |    6 -
 fs/ext2/super.c                           |    2 
 fs/ext4/super.c                           |   12 +-
 fs/f2fs/super.c                           |    6 -
 fs/fat/namei_msdos.c                      |    2 
 fs/fat/namei_vfat.c                       |    2 
 fs/freevxfs/vxfs_super.c                  |    2 
 fs/fuse/control.c                         |   12 +-
 fs/fuse/inode.c                           |   19 +--
 fs/fuse/virtio_fs.c                       |   21 ++-
 fs/gfs2/ops_fstype.c                      |   11 --
 fs/hfs/super.c                            |    2 
 fs/hfsplus/super.c                        |    2 
 fs/hostfs/hostfs_kern.c                   |    5 
 fs/hpfs/super.c                           |    2 
 fs/hugetlbfs/inode.c                      |    2 
 fs/isofs/inode.c                          |    2 
 fs/jffs2/super.c                          |   22 ++--
 fs/jfs/super.c                            |    2 
 fs/kernfs/mount.c                         |   20 +--
 fs/minix/inode.c                          |    2 
 fs/nfs/client.c                           |    2 
 fs/nfs/fs_context.c                       |   19 +++
 fs/nfs/internal.h                         |    1 
 fs/nfs/nfs4proc.c                         |    8 -
 fs/nfs/nfs4trace.h                        |    6 -
 fs/nfs/nfs4xdr.c                          |    2 
 fs/nfs/super.c                            |   26 ----
 fs/nfs/sysfs.h                            |    2 
 fs/nfsd/nfsctl.c                          |   22 ++--
 fs/nilfs2/super.c                         |    2 
 fs/nsfs.c                                 |    1 
 fs/ntfs/super.c                           |    2 
 fs/ntfs3/super.c                          |    6 -
 fs/ocfs2/dlmfs/dlmfs.c                    |    2 
 fs/ocfs2/super.c                          |    2 
 fs/omfs/inode.c                           |    2 
 fs/openpromfs/inode.c                     |    1 
 fs/orangefs/orangefs-kernel.h             |    2 
 fs/orangefs/orangefs-mod.c                |    2 
 fs/orangefs/super.c                       |   13 --
 fs/overlayfs/super.c                      |    1 
 fs/pipe.c                                 |    1 
 fs/proc/root.c                            |   16 +--
 fs/pstore/inode.c                         |    8 -
 fs/qnx4/inode.c                           |    7 -
 fs/qnx6/inode.c                           |    2 
 fs/ramfs/inode.c                          |    6 -
 fs/reiserfs/super.c                       |    7 -
 fs/romfs/super.c                          |    6 -
 fs/smb/client/cifsfs.c                    |   17 ++-
 fs/squashfs/super.c                       |    2 
 fs/super.c                                |  159 ++++++++++++++++--------------
 fs/sysfs/mount.c                          |    7 -
 fs/sysv/super.c                           |    4 
 fs/tracefs/inode.c                        |    2 
 fs/ubifs/super.c                          |   10 -
 fs/udf/super.c                            |    2 
 fs/ufs/super.c                            |    2 
 fs/vboxsf/super.c                         |    1 
 fs/xfs/xfs_buf.c                          |    2 
 fs/xfs/xfs_super.c                        |    6 -
 fs/zonefs/super.c                         |   34 ++----
 include/linux/fs.h                        |   11 --
 include/linux/kernfs.h                    |    5 
 include/linux/mtd/super.h                 |    2 
 include/linux/nfs_fs_sb.h                 |    1 
 include/linux/ramfs.h                     |    2 
 init/do_mounts.c                          |    6 -
 ipc/mqueue.c                              |    2 
 kernel/bpf/inode.c                        |    2 
 kernel/cgroup/cgroup.c                    |   10 +
 kernel/resource.c                         |    1 
 mm/secretmem.c                            |    1 
 mm/shmem.c                                |    5 
 net/socket.c                              |    1 
 net/sunrpc/rpc_pipe.c                     |   19 ++-
 security/apparmor/apparmorfs.c            |    1 
 security/inode.c                          |    8 -
 security/selinux/selinuxfs.c              |   15 --
 security/smack/smackfs.c                  |    6 -
 126 files changed, 524 insertions(+), 679 deletions(-)

[PATCH 01/19] fs: reflow deactivate_locked_super

[Christoph Hellwig]

Return early for the case where the super block isn't cleaned up to
reduce level of indentation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/super.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 2d762ce67f6e6c..127a17d958a482 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -476,27 +476,28 @@ static void kill_super_notify(struct super_block *sb)
 void deactivate_locked_super(struct super_block *s)
 {
 	struct file_system_type *fs = s->s_type;
-	if (atomic_dec_and_test(&s->s_active)) {
-		unregister_shrinker(&s->s_shrink);
-		fs->kill_sb(s);
 
-		kill_super_notify(s);
-
-		/*
-		 * Since list_lru_destroy() may sleep, we cannot call it from
-		 * put_super(), where we hold the sb_lock. Therefore we destroy
-		 * the lru lists right now.
-		 */
-		list_lru_destroy(&s->s_dentry_lru);
-		list_lru_destroy(&s->s_inode_lru);
-
-		put_filesystem(fs);
-		put_super(s);
-	} else {
+	if (!atomic_dec_and_test(&s->s_active)) {
 		super_unlock_excl(s);
+		return;
 	}
-}
 
+	unregister_shrinker(&s->s_shrink);
+	fs->kill_sb(s);
+
+	kill_super_notify(s);
+
+	/*
+	 * Since list_lru_destroy() may sleep, we cannot call it from
+	 * put_super(), where we hold the sb_lock. Therefore we destroy
+	 * the lru lists right now.
+	 */
+	list_lru_destroy(&s->s_dentry_lru);
+	list_lru_destroy(&s->s_inode_lru);
+
+	put_filesystem(fs);
+	put_super(s);
+}
 EXPORT_SYMBOL(deactivate_locked_super);
 
 /**
-- 
2.39.2

Re: [PATCH 01/19] fs: reflow deactivate_locked_super

[Christian Brauner]

On Wed, Sep 13, 2023 at 08:09:55AM -0300, Christoph Hellwig wrote:
> Return early for the case where the super block isn't cleaned up to
> reduce level of indentation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/super.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 2d762ce67f6e6c..127a17d958a482 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -476,27 +476,28 @@ static void kill_super_notify(struct super_block *sb)
>  void deactivate_locked_super(struct super_block *s)

I wouldn't mind s/s/sb/ here as well. So we stop using @s in some and
@sb in other places.

Otherwise looks good to me,
Reviewed-by: Christian Brauner <brauner@kernel.org>

Re: [PATCH 01/19] fs: reflow deactivate_locked_super

[Christoph Hellwig]

On Wed, Sep 13, 2023 at 06:35:03PM +0200, Christian Brauner wrote:
> I wouldn't mind s/s/sb/ here as well. So we stop using @s in some and
> @sb in other places.

I did that in an earlier version and decided to have some less churn.
But I can add it back.

[PATCH 02/19] fs: make ->kill_sb optional

[Christoph Hellwig]

Call generic_shutdown_super if ->kill_sb is not provided by the file
system.  This can't currently happen but will become common soon.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/super.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/super.c b/fs/super.c
index 127a17d958a482..ab234e6af48605 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -483,7 +483,11 @@ void deactivate_locked_super(struct super_block *s)
 	}
 
 	unregister_shrinker(&s->s_shrink);
-	fs->kill_sb(s);
+
+	if (fs->kill_sb)
+		fs->kill_sb(s);
+	else
+		generic_shutdown_super(s);
 
 	kill_super_notify(s);
 
-- 
2.39.2

[PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christoph Hellwig]

Releasing an anon dev_t is a very common thing when freeing a
super_block, as that's done for basically any not block based file
system (modulo the odd mtd special case).  So instead of requiring
a special ->kill_sb helper and a lot of boilerplate in more complicated
file systems, just release the anon dev_t in deactivate_locked_super if
the super_block was using one.

As the freeing is done after the main call to kill_super_notify, this
removes the need for having two slightly different call sites for it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bdev.c                    |  1 -
 drivers/dax/super.c             |  1 -
 drivers/dma-buf/dma-buf.c       |  1 -
 drivers/gpu/drm/drm_drv.c       |  1 -
 drivers/misc/cxl/api.c          |  1 -
 drivers/scsi/cxlflash/ocxl_hw.c |  1 -
 fs/9p/vfs_super.c               |  2 +-
 fs/afs/super.c                  |  2 +-
 fs/aio.c                        |  1 -
 fs/anon_inodes.c                |  1 -
 fs/autofs/inode.c               |  4 ++--
 fs/btrfs/super.c                |  3 ++-
 fs/btrfs/tests/btrfs-tests.c    |  1 -
 fs/ceph/super.c                 |  2 +-
 fs/coda/inode.c                 |  1 -
 fs/ecryptfs/main.c              |  3 ++-
 fs/erofs/super.c                |  4 ++--
 fs/fuse/inode.c                 |  2 +-
 fs/fuse/virtio_fs.c             |  2 +-
 fs/hostfs/hostfs_kern.c         |  2 +-
 fs/kernfs/mount.c               |  2 +-
 fs/nfs/super.c                  |  2 +-
 fs/nsfs.c                       |  1 -
 fs/openpromfs/inode.c           |  1 -
 fs/orangefs/super.c             |  2 +-
 fs/overlayfs/super.c            |  1 -
 fs/pipe.c                       |  1 -
 fs/proc/root.c                  |  4 ++--
 fs/smb/client/cifsfs.c          |  2 +-
 fs/super.c                      | 22 ++++++++--------------
 fs/ubifs/super.c                |  3 ++-
 fs/vboxsf/super.c               |  1 -
 include/linux/fs.h              |  1 -
 kernel/resource.c               |  1 -
 mm/secretmem.c                  |  1 -
 net/socket.c                    |  1 -
 security/apparmor/apparmorfs.c  |  1 -
 37 files changed, 30 insertions(+), 53 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index f3b13aa1b7d428..9db691401497bb 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -358,7 +358,6 @@ static int bd_init_fs_context(struct fs_context *fc)
 static struct file_system_type bd_type = {
 	.name		= "bdev",
 	.init_fs_context = bd_init_fs_context,
-	.kill_sb	= kill_anon_super,
 };
 
 struct super_block *blockdev_superblock __read_mostly;
diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 0da9232ea1754b..a9315b7396e68a 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -397,7 +397,6 @@ static int dax_init_fs_context(struct fs_context *fc)
 static struct file_system_type dax_fs_type = {
 	.name		= "dax",
 	.init_fs_context = dax_init_fs_context,
-	.kill_sb	= kill_anon_super,
 };
 
 static int dax_test(struct inode *inode, void *data)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 21916bba77d58b..7313e99f6e8ea5 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -125,7 +125,6 @@ static int dma_buf_fs_init_context(struct fs_context *fc)
 static struct file_system_type dma_buf_fs_type = {
 	.name = "dmabuf",
 	.init_fs_context = dma_buf_fs_init_context,
-	.kill_sb = kill_anon_super,
 };
 
 static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 3eda026ffac6a9..83676229cbe233 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -530,7 +530,6 @@ static struct file_system_type drm_fs_type = {
 	.name		= "drm",
 	.owner		= THIS_MODULE,
 	.init_fs_context = drm_fs_init_fs_context,
-	.kill_sb	= kill_anon_super,
 };
 
 static struct inode *drm_fs_inode_new(void)
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index d85c5653086357..05b40076a0b481 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -44,7 +44,6 @@ static struct file_system_type cxl_fs_type = {
 	.name		= "cxl",
 	.owner		= THIS_MODULE,
 	.init_fs_context = cxl_fs_init_fs_context,
-	.kill_sb	= kill_anon_super,
 };
 
 
diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
index 6542818e595a64..20f22610b104df 100644
--- a/drivers/scsi/cxlflash/ocxl_hw.c
+++ b/drivers/scsi/cxlflash/ocxl_hw.c
@@ -43,7 +43,6 @@ static struct file_system_type ocxlflash_fs_type = {
 	.name		= "ocxlflash",
 	.owner		= THIS_MODULE,
 	.init_fs_context = ocxlflash_fs_init_fs_context,
-	.kill_sb	= kill_anon_super,
 };
 
 /*
diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 73db55c050bf10..9e60eddf5179ed 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -217,7 +217,7 @@ static void v9fs_kill_super(struct super_block *s)
 
 	p9_debug(P9_DEBUG_VFS, " %p\n", s);
 
-	kill_anon_super(s);
+	generic_shutdown_super(s);
 
 	v9fs_session_cancel(v9ses);
 	v9fs_session_close(v9ses);
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 95d713074dc813..754b9828233497 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -544,7 +544,7 @@ static void afs_kill_super(struct super_block *sb)
 	 */
 	if (as->volume)
 		rcu_assign_pointer(as->volume->sb, NULL);
-	kill_anon_super(sb);
+	generic_shutdown_super(sb);
 	if (as->volume)
 		afs_deactivate_volume(as->volume);
 	afs_destroy_sbi(as);
diff --git a/fs/aio.c b/fs/aio.c
index a4c2a6bac72ce9..de56e4a880debe 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -293,7 +293,6 @@ static int __init aio_setup(void)
 	static struct file_system_type aio_fs = {
 		.name		= "aio",
 		.init_fs_context = aio_init_fs_context,
-		.kill_sb	= kill_anon_super,
 	};
 	aio_mnt = kern_mount(&aio_fs);
 	if (IS_ERR(aio_mnt))
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 24192a7667edf7..9c670bbe0f62ce 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -52,7 +52,6 @@ static int anon_inodefs_init_fs_context(struct fs_context *fc)
 static struct file_system_type anon_inode_fs_type = {
 	.name		= "anon_inodefs",
 	.init_fs_context = anon_inodefs_init_fs_context,
-	.kill_sb	= kill_anon_super,
 };
 
 static struct inode *anon_inode_make_secure_inode(
diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 2b49662ed237de..c3b64799155840 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -44,7 +44,7 @@ void autofs_kill_sb(struct super_block *sb)
 	/*
 	 * In the event of a failure in get_sb_nodev the superblock
 	 * info is not present so nothing else has been setup, so
-	 * just call kill_anon_super when we are called from
+	 * just call generic_shutdown_super when we are called from
 	 * deactivate_super.
 	 */
 	if (sbi) {
@@ -54,7 +54,7 @@ void autofs_kill_sb(struct super_block *sb)
 	}
 
 	pr_debug("shutting down\n");
-	kill_litter_super(sb);
+	generic_shutdown_super(sb);
 	if (sbi)
 		kfree_rcu(sbi, rcu);
 }
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 09bfe68d2ea3fc..01b86bd4eae8dc 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2141,7 +2141,8 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 static void btrfs_kill_super(struct super_block *sb)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(sb);
-	kill_anon_super(sb);
+
+	generic_shutdown_super(sb);
 	btrfs_free_fs_info(fs_info);
 }
 
diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
index ca09cf9afce800..c30280376fc32e 100644
--- a/fs/btrfs/tests/btrfs-tests.c
+++ b/fs/btrfs/tests/btrfs-tests.c
@@ -48,7 +48,6 @@ static int btrfs_test_init_fs_context(struct fs_context *fc)
 static struct file_system_type test_type = {
 	.name		= "btrfs_test_fs",
 	.init_fs_context = btrfs_test_init_fs_context,
-	.kill_sb	= kill_anon_super,
 };
 
 struct inode *btrfs_new_test_inode(void)
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 2d7f5a8d4a9260..7feef0b35b97b5 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1557,7 +1557,7 @@ static void ceph_kill_sb(struct super_block *s)
 	}
 
 	mdsc->stopping = CEPH_MDSC_STOPPING_FLUSHED;
-	kill_anon_super(s);
+	generic_shutdown_super(s);
 
 	fsc->client->extra_mon_dispatch = NULL;
 	ceph_fs_debugfs_cleanup(fsc);
diff --git a/fs/coda/inode.c b/fs/coda/inode.c
index 0c7c2528791ebc..2d4ee3c7e8654b 100644
--- a/fs/coda/inode.c
+++ b/fs/coda/inode.c
@@ -325,7 +325,6 @@ struct file_system_type coda_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "coda",
 	.mount		= coda_mount,
-	.kill_sb	= kill_anon_super,
 	.fs_flags	= FS_BINARY_MOUNTDATA,
 };
 MODULE_ALIAS_FS("coda");
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 2dc927ba067fec..d99b2311759166 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -625,7 +625,8 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
 static void ecryptfs_kill_block_super(struct super_block *sb)
 {
 	struct ecryptfs_sb_info *sb_info = ecryptfs_superblock_to_private(sb);
-	kill_anon_super(sb);
+
+	generic_shutdown_super(sb);
 	if (!sb_info)
 		return;
 	ecryptfs_destroy_mount_crypt_stat(&sb_info->mount_crypt_stat);
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 44a24d573f1fd3..07c36ccf454e53 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -878,12 +878,12 @@ static void erofs_kill_sb(struct super_block *sb)
 
 	/* pseudo mount for anon inodes */
 	if (sb->s_flags & SB_KERNMOUNT) {
-		kill_anon_super(sb);
+		generic_shutdown_super(sb);
 		return;
 	}
 
 	if (erofs_is_fscache_mode(sb))
-		kill_anon_super(sb);
+		generic_shutdown_super(sb);
 	else
 		kill_block_super(sb);
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 2e4eb7cf26fb33..42523edb32fd53 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1840,7 +1840,7 @@ EXPORT_SYMBOL(fuse_mount_destroy);
 static void fuse_kill_sb_anon(struct super_block *sb)
 {
 	fuse_sb_destroy(sb);
-	kill_anon_super(sb);
+	generic_shutdown_super(sb);
 	fuse_mount_destroy(get_fuse_mount_super(sb));
 }
 
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 5f1be1da92ce94..0a0d593e5a9c79 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1398,7 +1398,7 @@ static void virtio_kill_sb(struct super_block *sb)
 		if (last)
 			virtio_fs_conn_destroy(fm);
 	}
-	kill_anon_super(sb);
+	generic_shutdown_super(sb);
 	fuse_mount_destroy(fm);
 }
 
diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
index dc5a5cea5fae41..97f3c9709418c9 100644
--- a/fs/hostfs/hostfs_kern.c
+++ b/fs/hostfs/hostfs_kern.c
@@ -981,7 +981,7 @@ static struct dentry *hostfs_read_sb(struct file_system_type *type,
 
 static void hostfs_kill_sb(struct super_block *s)
 {
-	kill_anon_super(s);
+	generic_shutdown_super(s);
 	kfree(s->s_fs_info);
 }
 
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index c4bf26142eec9b..772d059d4054ec 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -399,7 +399,7 @@ void kernfs_kill_sb(struct super_block *sb)
 	 * Remove the superblock from fs_supers/s_instances
 	 * so we can't find it, before freeing kernfs_super_info.
 	 */
-	kill_anon_super(sb);
+	generic_shutdown_super(sb);
 	kfree(info);
 }
 
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 0d6473cb00cb3e..29d6a55b9d400d 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1341,7 +1341,7 @@ void nfs_kill_super(struct super_block *s)
 	struct nfs_server *server = NFS_SB(s);
 
 	nfs_sysfs_move_sb_to_server(server);
-	kill_anon_super(s);
+	generic_shutdown_super(s);
 
 	nfs_fscache_release_super_cookie(s);
 
diff --git a/fs/nsfs.c b/fs/nsfs.c
index 647a22433bd8a9..d5fb7c00fe21f1 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -277,7 +277,6 @@ static int nsfs_init_fs_context(struct fs_context *fc)
 static struct file_system_type nsfs = {
 	.name = "nsfs",
 	.init_fs_context = nsfs_init_fs_context,
-	.kill_sb = kill_anon_super,
 };
 
 void __init nsfs_init(void)
diff --git a/fs/openpromfs/inode.c b/fs/openpromfs/inode.c
index b2457cb97fa008..3b51dd926d11c8 100644
--- a/fs/openpromfs/inode.c
+++ b/fs/openpromfs/inode.c
@@ -427,7 +427,6 @@ static struct file_system_type openprom_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "openpromfs",
 	.init_fs_context = openpromfs_init_fs_context,
-	.kill_sb	= kill_anon_super,
 };
 MODULE_ALIAS_FS("openpromfs");
 
diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index 5254256a224d7a..42cb3e9b1effee 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -602,7 +602,7 @@ void orangefs_kill_sb(struct super_block *sb)
 	gossip_debug(GOSSIP_SUPER_DEBUG, "orangefs_kill_sb: called\n");
 
 	/* provided sb cleanup */
-	kill_anon_super(sb);
+	generic_shutdown_super(sb);
 
 	if (!ORANGEFS_SB(sb)) {
 		mutex_lock(&orangefs_request_mutex);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index def266b5e2a33b..194cf18787496d 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1507,7 +1507,6 @@ struct file_system_type ovl_fs_type = {
 	.init_fs_context	= ovl_init_fs_context,
 	.parameters		= ovl_parameter_spec,
 	.fs_flags		= FS_USERNS_MOUNT,
-	.kill_sb		= kill_anon_super,
 };
 MODULE_ALIAS_FS("overlay");
 
diff --git a/fs/pipe.c b/fs/pipe.c
index 6c1a9b1db9076c..858e8b19d78527 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -1439,7 +1439,6 @@ static int pipefs_init_fs_context(struct fs_context *fc)
 static struct file_system_type pipe_fs_type = {
 	.name		= "pipefs",
 	.init_fs_context = pipefs_init_fs_context,
-	.kill_sb	= kill_anon_super,
 };
 
 #ifdef CONFIG_SYSCTL
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 9191248f2dacb4..2282366449ac0b 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -262,14 +262,14 @@ static void proc_kill_sb(struct super_block *sb)
 	struct proc_fs_info *fs_info = proc_sb_info(sb);
 
 	if (!fs_info) {
-		kill_anon_super(sb);
+		generic_shutdown_super(sb);
 		return;
 	}
 
 	dput(fs_info->proc_self);
 	dput(fs_info->proc_thread_self);
 
-	kill_anon_super(sb);
+	generic_shutdown_super(sb);
 	put_pid_ns(fs_info->pid_ns);
 	kfree(fs_info);
 }
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 22869cda13565e..fb792acffeca37 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -302,7 +302,7 @@ static void cifs_kill_sb(struct super_block *sb)
 		cifs_sb->root = NULL;
 	}
 
-	kill_anon_super(sb);
+	generic_shutdown_super(sb);
 	cifs_umount(cifs_sb);
 }
 
diff --git a/fs/super.c b/fs/super.c
index ab234e6af48605..bbe55f0651cca4 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -438,10 +438,6 @@ static void kill_super_notify(struct super_block *sb)
 {
 	lockdep_assert_not_held(&sb->s_umount);
 
-	/* already notified earlier */
-	if (sb->s_flags & SB_DEAD)
-		return;
-
 	/*
 	 * Remove it from @fs_supers so it isn't found by new
 	 * sget{_fc}() walkers anymore. Any concurrent mounter still
@@ -491,6 +487,13 @@ void deactivate_locked_super(struct super_block *s)
 
 	kill_super_notify(s);
 
+	/*
+	 * If the super_block was using an anon dev_t, release it now that we've
+	 * notified everyone that the super_block is going away.
+	 */
+	if (s->s_dev && MAJOR(s->s_dev) == 0)
+		free_anon_bdev(s->s_dev);
+
 	/*
 	 * Since list_lru_destroy() may sleep, we cannot call it from
 	 * put_super(), where we hold the sb_lock. Therefore we destroy
@@ -1291,20 +1294,11 @@ int set_anon_super(struct super_block *s, void *data)
 }
 EXPORT_SYMBOL(set_anon_super);
 
-void kill_anon_super(struct super_block *sb)
-{
-	dev_t dev = sb->s_dev;
-	generic_shutdown_super(sb);
-	kill_super_notify(sb);
-	free_anon_bdev(dev);
-}
-EXPORT_SYMBOL(kill_anon_super);
-
 void kill_litter_super(struct super_block *sb)
 {
 	if (sb->s_root)
 		d_genocide(sb->s_root);
-	kill_anon_super(sb);
+	generic_shutdown_super(sb);
 }
 EXPORT_SYMBOL(kill_litter_super);
 
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index b08fb28d16b55b..6527175591a729 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2350,7 +2350,8 @@ static struct dentry *ubifs_mount(struct file_system_type *fs_type, int flags,
 static void kill_ubifs_super(struct super_block *s)
 {
 	struct ubifs_info *c = s->s_fs_info;
-	kill_anon_super(s);
+
+	generic_shutdown_super(s);
 	kfree(c);
 }
 
diff --git a/fs/vboxsf/super.c b/fs/vboxsf/super.c
index 1fb8f4df60cbb3..aa7e627bf9bd4b 100644
--- a/fs/vboxsf/super.c
+++ b/fs/vboxsf/super.c
@@ -450,7 +450,6 @@ static struct file_system_type vboxsf_fs_type = {
 	.owner			= THIS_MODULE,
 	.name			= "vboxsf",
 	.init_fs_context	= vboxsf_init_fs_context,
-	.kill_sb		= kill_anon_super
 };
 
 /* Module initialization/finalization handlers */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4aeb3fa1192771..129b8c0c83960b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2382,7 +2382,6 @@ extern struct dentry *mount_subtree(struct vfsmount *mnt, const char *path);
 void retire_super(struct super_block *sb);
 void generic_shutdown_super(struct super_block *sb);
 void kill_block_super(struct super_block *sb);
-void kill_anon_super(struct super_block *sb);
 void kill_litter_super(struct super_block *sb);
 void deactivate_super(struct super_block *sb);
 void deactivate_locked_super(struct super_block *sb);
diff --git a/kernel/resource.c b/kernel/resource.c
index b1763b2fd7ef3e..fde412517ef0cc 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1979,7 +1979,6 @@ static struct file_system_type iomem_fs_type = {
 	.name		= "iomem",
 	.owner		= THIS_MODULE,
 	.init_fs_context = iomem_fs_init_fs_context,
-	.kill_sb	= kill_anon_super,
 };
 
 static int __init iomem_init_inode(void)
diff --git a/mm/secretmem.c b/mm/secretmem.c
index 3afb5ad701e14a..74e1cdb1317cd7 100644
--- a/mm/secretmem.c
+++ b/mm/secretmem.c
@@ -275,7 +275,6 @@ static int secretmem_init_fs_context(struct fs_context *fc)
 static struct file_system_type secretmem_fs = {
 	.name		= "secretmem",
 	.init_fs_context = secretmem_init_fs_context,
-	.kill_sb	= kill_anon_super,
 };
 
 static int __init secretmem_init(void)
diff --git a/net/socket.c b/net/socket.c
index c8b08b32f097ec..a137b08a7d94d1 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -425,7 +425,6 @@ static struct vfsmount *sock_mnt __read_mostly;
 static struct file_system_type sock_fs_type = {
 	.name =		"sockfs",
 	.init_fs_context = sockfs_init_fs_context,
-	.kill_sb =	kill_anon_super,
 };
 
 /*
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index bd6a910f65282a..eceb5443842cdf 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -198,7 +198,6 @@ static struct file_system_type aafs_ops = {
 	.owner = THIS_MODULE,
 	.name = AAFS_NAME,
 	.init_fs_context = apparmorfs_init_fs_context,
-	.kill_sb = kill_anon_super,
 };
 
 /**
-- 
2.39.2

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Al Viro]

On Wed, Sep 13, 2023 at 08:09:57AM -0300, Christoph Hellwig wrote:
> Releasing an anon dev_t is a very common thing when freeing a
> super_block, as that's done for basically any not block based file
> system (modulo the odd mtd special case).  So instead of requiring
> a special ->kill_sb helper and a lot of boilerplate in more complicated
> file systems, just release the anon dev_t in deactivate_locked_super if
> the super_block was using one.
> 
> As the freeing is done after the main call to kill_super_notify, this
> removes the need for having two slightly different call sites for it.

Huh?  At this stage in your series freeing is still in ->kill_sb()
instances, after the calls of kill_anon_super() you've turned into
the calls of generic_shutdown_super().

You do split it off into a separate method later in the series, but
at this point you are reopening the same UAF that had been dealt with
in dc3216b14160 "super: ensure valid info".

Either move the introduction of ->free_sb() before that one, or
split it into lifting put_anon_bdev() (left here) and getting rid
of kill_anon_super() (after ->free_sb() introduction).

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Al Viro]

On Thu, Sep 14, 2023 at 12:27:12AM +0100, Al Viro wrote:
> On Wed, Sep 13, 2023 at 08:09:57AM -0300, Christoph Hellwig wrote:
> > Releasing an anon dev_t is a very common thing when freeing a
> > super_block, as that's done for basically any not block based file
> > system (modulo the odd mtd special case).  So instead of requiring
> > a special ->kill_sb helper and a lot of boilerplate in more complicated
> > file systems, just release the anon dev_t in deactivate_locked_super if
> > the super_block was using one.
> > 
> > As the freeing is done after the main call to kill_super_notify, this
> > removes the need for having two slightly different call sites for it.
> 
> Huh?  At this stage in your series freeing is still in ->kill_sb()
> instances, after the calls of kill_anon_super() you've turned into
> the calls of generic_shutdown_super().
> 
> You do split it off into a separate method later in the series, but
> at this point you are reopening the same UAF that had been dealt with
> in dc3216b14160 "super: ensure valid info".
> 
> Either move the introduction of ->free_sb() before that one, or
> split it into lifting put_anon_bdev() (left here) and getting rid
> of kill_anon_super() (after ->free_sb() introduction).

Actually, looking at the final stage in the series, you still have
kill_super_notify() done *AFTER* ->free_sb() call.  So the problem
persists until the very end...

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Al Viro]

On Thu, Sep 14, 2023 at 03:37:05AM +0100, Al Viro wrote:
> On Thu, Sep 14, 2023 at 12:27:12AM +0100, Al Viro wrote:
> > On Wed, Sep 13, 2023 at 08:09:57AM -0300, Christoph Hellwig wrote:
> > > Releasing an anon dev_t is a very common thing when freeing a
> > > super_block, as that's done for basically any not block based file
> > > system (modulo the odd mtd special case).  So instead of requiring
> > > a special ->kill_sb helper and a lot of boilerplate in more complicated
> > > file systems, just release the anon dev_t in deactivate_locked_super if
> > > the super_block was using one.
> > > 
> > > As the freeing is done after the main call to kill_super_notify, this
> > > removes the need for having two slightly different call sites for it.
> > 
> > Huh?  At this stage in your series freeing is still in ->kill_sb()
> > instances, after the calls of kill_anon_super() you've turned into
> > the calls of generic_shutdown_super().
> > 
> > You do split it off into a separate method later in the series, but
> > at this point you are reopening the same UAF that had been dealt with
> > in dc3216b14160 "super: ensure valid info".
> > 
> > Either move the introduction of ->free_sb() before that one, or
> > split it into lifting put_anon_bdev() (left here) and getting rid
> > of kill_anon_super() (after ->free_sb() introduction).
> 
> Actually, looking at the final stage in the series, you still have
> kill_super_notify() done *AFTER* ->free_sb() call.  So the problem
> persists until the very end...

It's worse - look at the rationale for 2c18a63b760a "super: wait until
we passed kill super".  Basically, "don't remove from the lists
until after block device closing".  IOW, we have

* stuff that needs to be done before generic_shutdown_super() (things
like pinned dentries on ramfs, etc.)
* generic_shutdown_super() itself (dentry/inode eviction, optionally
->put_super())
* stuff that needs to be done before eviction from the lists (block
device closing, since 2c18a63b760a)
* eviction from the lists
* stuff that needs to be done *after* eviction from the lists.

BTW, this part of commit message in 2c18a63b760a is rather confused:
    Recent rework moved block device closing out of sb->put_super() and into
    sb->kill_sb() to avoid deadlocks as s_umount is held in put_super() and
    blkdev_put() can end up taking s_umount again.

That was *NOT* what a recent rework had done.  Block device closing had never
been inside ->put_super() - at no point since that (closing, that is) had been
introduced back in 0.97 ;-)  ->put_super() predates it (0.95c+).

The race is real, but the cause is not some kind of move of blkdev_put().
Your 2ea6f68932f7 "fs: use the super_block as holder when mounting file
systems" is where it actually came from.

Christoph, could you explain what the hell do we need that for?  It does
create the race in question and AFAICS 2c18a63b760a (and followups trying
to plug holes in it) had been nothing but headache.

Old logics: if mount attempt with a different fs type happens, -EBUSY
is precisely corrent - we would've gotten just that if mount() came
before umount().  If the type matches, we might
	1) come before deactivate_locked_super() by umount(2).
No problem, we succeed.
	2) come after the beginning of shutdown, but before the
removal from the list; fine, we'll wait for the sucker to be
unlocked (which happens in the end of generic_shutdown_super()),
notice it's dead and create a new superblock.  Since the only
part left on the umount side is closing the device, we are
just fine.
	3) come after the removal from the list.  So we won't
wait for the old superblock to be unlocked, other than that
it's exactly the same as (2).  It doesn't matter whether we
open the device before or after close by umount - same owner
anyway, no -EBUSY.

Your "owner shall be the superblock" breaks that...

If you want to mess with _three_-way split of ->kill_sb(),
please start with writing down the rules re what should
go into each of those parts; such writeup should go into
Documentation/filesystems/porting anyway, even if the
split is a two-way one, BTW.

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christian Brauner]

> BTW, this part of commit message in 2c18a63b760a is rather confused:
>     Recent rework moved block device closing out of sb->put_super() and into
>     sb->kill_sb() to avoid deadlocks as s_umount is held in put_super() and
>     blkdev_put() can end up taking s_umount again.
> 
> That was *NOT* what a recent rework had done.  Block device closing had never
> been inside ->put_super() - at no point since that (closing, that is) had been
> introduced back in 0.97 ;-)  ->put_super() predates it (0.95c+).

I think the commit message probably just isn't clear enough. The main
block device of a superblock isn't closed in sb->put_super(). That's
always been closed in kill_block_super() after generic_shutdown_super().

But afaict filesystem like ext4 and xfs may have additional block
devices open exclusively and closed them in sb->put_super():

xfs_fs_put_super()
-> xfs_close_devices()
   -> xfs_blkdev_put()
      -> blkdev_put()

ext4_put_super()
-> ext4_blkdev_remove()
   -> blkdev_put()

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christoph Hellwig]

On Thu, Sep 14, 2023 at 09:56:57AM +0200, Christian Brauner wrote:
> > BTW, this part of commit message in 2c18a63b760a is rather confused:
> >     Recent rework moved block device closing out of sb->put_super() and into
> >     sb->kill_sb() to avoid deadlocks as s_umount is held in put_super() and
> >     blkdev_put() can end up taking s_umount again.
> > 
> > That was *NOT* what a recent rework had done.  Block device closing had never
> > been inside ->put_super() - at no point since that (closing, that is) had been
> > introduced back in 0.97 ;-)  ->put_super() predates it (0.95c+).
> 
> I think the commit message probably just isn't clear enough. The main
> block device of a superblock isn't closed in sb->put_super(). That's
> always been closed in kill_block_super() after generic_shutdown_super().

Yes.

> But afaict filesystem like ext4 and xfs may have additional block
> devices open exclusively and closed them in sb->put_super():
> 
> xfs_fs_put_super()
> -> xfs_close_devices()
>    -> xfs_blkdev_put()
>       -> blkdev_put()
> 
> ext4_put_super()
> -> ext4_blkdev_remove()
>    -> blkdev_put()

Yes.

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christian Brauner]

> Christoph, could you explain what the hell do we need that for?  It does
> create the race in question and AFAICS 2c18a63b760a (and followups trying
> to plug holes in it) had been nothing but headache.
> 
> Old logics: if mount attempt with a different fs type happens, -EBUSY
> is precisely corrent - we would've gotten just that if mount() came
> before umount().  If the type matches, we might
> 	1) come before deactivate_locked_super() by umount(2).
> No problem, we succeed.
> 	2) come after the beginning of shutdown, but before the
> removal from the list; fine, we'll wait for the sucker to be
> unlocked (which happens in the end of generic_shutdown_super()),
> notice it's dead and create a new superblock.  Since the only
> part left on the umount side is closing the device, we are
> just fine.
> 	3) come after the removal from the list.  So we won't
> wait for the old superblock to be unlocked, other than that
> it's exactly the same as (2).  It doesn't matter whether we
> open the device before or after close by umount - same owner
> anyway, no -EBUSY.
> 
> Your "owner shall be the superblock" breaks that...
> 
> If you want to mess with _three_-way split of ->kill_sb(),
> please start with writing down the rules re what should
> go into each of those parts; such writeup should go into
> Documentation/filesystems/porting anyway, even if the
> split is a two-way one, BTW.

Hm, I think that characterization of Christoph's changes is a bit harsh.

Yes, you're right that making the superblock and not the filesytem type
the bd_holder changes the logic and we are aware of that of course. And
it requires changes such as moving additional block device closing from
where some callers currently do it.

But the filesytem type is not a very useful holder itself and has other
drawbacks. The obvious one being that it requires us to wade through all
superblocks on the system trying to find the superblock associated with
a given block device continously grabbing and dropping sb_lock and
s_umount. None of that is very pleasant nor elegant and it is for sure
not very easy to understand (Plus, it's broken for btrfs freezing and
syncing via block level ioctls.).

Using the superblock as holder makes this go away and is overall a lot
more useful and intuitive and can be extended to filesystems with
multiple devices (Of which we apparently are bound to get more.).

So I think this change is worth the pain.

It's a fair point that these lifetime rules should be documented in
Documentation/filesystems/. The old lifetime documentation is too sparse
to be useful though.

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Al Viro]

On Thu, Sep 14, 2023 at 04:02:25PM +0200, Christian Brauner wrote:

> Yes, you're right that making the superblock and not the filesytem type
> the bd_holder changes the logic and we are aware of that of course. And
> it requires changes such as moving additional block device closing from
> where some callers currently do it.

Details, please?

> But the filesytem type is not a very useful holder itself and has other
> drawbacks. The obvious one being that it requires us to wade through all
> superblocks on the system trying to find the superblock associated with
> a given block device continously grabbing and dropping sb_lock and
> s_umount. None of that is very pleasant nor elegant and it is for sure
> not very easy to understand (Plus, it's broken for btrfs freezing and
> syncing via block level ioctls.).

"Constantly" is a bit of a stretch - IIRC, we grabbed sb_lock once, then
went through the list comparing ->s_bdev (without any extra locking),
then bumped ->s_count on the found superblock, dropped sb_lock,
grabbed ->s_umount on the sucker and verified it's still alive.

Repeated grabbing of any lock happened only on a race with fs shutdown;
normal case is one spin_lock, one spin_unlock, one down_read().

Oh, well...

> Using the superblock as holder makes this go away and is overall a lot
> more useful and intuitive and can be extended to filesystems with
> multiple devices (Of which we apparently are bound to get more.).
>
> So I think this change is worth the pain.
> 
> It's a fair point that these lifetime rules should be documented in
> Documentation/filesystems/. The old lifetime documentation is too sparse
> to be useful though.

What *are* these lifetime rules?  Seriously, you have 3 chunks of
fs-dependent actions at the moment:
	* the things needed to get rid of internal references pinning
inodes/dentries + whatever else we need done before generic_shutdown_super()
	* the stuff to be done between generic_shutdown_super() and
making the sucker invisible to sget()/sget_fc()
	* the stuff that must be done after we are sure that sget
callbacks won't be looking at this instance.

Note that Christoph's series has mashed (2) and (3) together, resulting
in UAF in a bunch of places.  And I'm dead serious about
Documentation/filesystems/porting being the right place; any development
tree of any filesystem (in-tree one or not) will have to go through the
changes and figure out WTF to do with their existing code.  We are
going to play whack-a-mole for at least several years as development
branches get rebased and merged.

Incidentally, I'm going to add a (belated by 10 years) chunk in porting.rst
re making sure that anything in superblock that might be needed by methods
called in RCU mode should *not* be freed without an RCU delay...  Should've
done that back in 3.12 merge window when RCU'd vfsmounts went in; as it
is, today we have several filesystems with exact same kind of breakage.
hfsplus and affs breakage had been there in 3.13 (missed those two), exfat
and ntfs3 - introduced later, by initial merges of filesystems in question.
Missed on review...

Hell knows - perhaps Documentation/filesystems/whack-a-mole might be a good
idea...

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Al Viro]

On Thu, Sep 14, 2023 at 05:58:05PM +0100, Al Viro wrote:

> Incidentally, I'm going to add a (belated by 10 years) chunk in porting.rst
> re making sure that anything in superblock that might be needed by methods
> called in RCU mode should *not* be freed without an RCU delay...  Should've
> done that back in 3.12 merge window when RCU'd vfsmounts went in; as it
> is, today we have several filesystems with exact same kind of breakage.
> hfsplus and affs breakage had been there in 3.13 (missed those two), exfat
> and ntfs3 - introduced later, by initial merges of filesystems in question.
> Missed on review...
> 
> Hell knows - perhaps Documentation/filesystems/whack-a-mole might be a good
> idea...

Actually, utf8 casefolding stuff also has the same problem, so ext4 and f2fs
with casefolding are also affected ;-/

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christian Brauner]

On Thu, Sep 14, 2023 at 08:23:31PM +0100, Al Viro wrote:
> On Thu, Sep 14, 2023 at 05:58:05PM +0100, Al Viro wrote:
> 
> > Incidentally, I'm going to add a (belated by 10 years) chunk in porting.rst
> > re making sure that anything in superblock that might be needed by methods
> > called in RCU mode should *not* be freed without an RCU delay...  Should've
> > done that back in 3.12 merge window when RCU'd vfsmounts went in; as it
> > is, today we have several filesystems with exact same kind of breakage.
> > hfsplus and affs breakage had been there in 3.13 (missed those two), exfat
> > and ntfs3 - introduced later, by initial merges of filesystems in question.
> > Missed on review...
> > 
> > Hell knows - perhaps Documentation/filesystems/whack-a-mole might be a good
> > idea...

pitfalls.rst or common-bugs.rst

or something like that.

> 
> Actually, utf8 casefolding stuff also has the same problem, so ext4 and f2fs
> with casefolding are also affected ;-/

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christian Brauner]

On Thu, Sep 14, 2023 at 05:58:05PM +0100, Al Viro wrote:
> On Thu, Sep 14, 2023 at 04:02:25PM +0200, Christian Brauner wrote:
> 
> > Yes, you're right that making the superblock and not the filesytem type
> > the bd_holder changes the logic and we are aware of that of course. And
> > it requires changes such as moving additional block device closing from
> > where some callers currently do it.
> 
> Details, please?

Filesystems like xfs and ext4 that closed additional block devices (For
example, the logdev= mount option for xfs.) in put_super() could go
through stuff like:

blkdev_put()
-> bdev->bd_disk->fops->release() == lo_release()
   -> __loop_clr_fd()
      -> disk_force_media_change()
         -> __invalidate_device()
            -> get_super()

which wouldn't have been a problem before because get_super() matched on
sb->s_bdev which obviously doesn't work because a log device or rt
device or whatever isn't the main block device. So we couldn't have
deadlocked.

But the fact that it is called in that manner from that place in the
first place is wildly adventurous especially considering that there
isn't __a single comment__ in that code why that is safe. So good luck
figuring this all out.

Now that we don't have to do that s_bdev matching thing anymore because
we directly associate the superblock with the block device we can go
straight from block device to superblock. But now calling blkdev_put()
under put_super() which holds s_umount could deadlock. So it's moved to
kill_sb where it should've always been called. Even without the
potential deadlock in the new scheme that's cleaner and easier to
understand imho and it just works for any block device.

> Note that Christoph's series has mashed (2) and (3) together, resulting
> in UAF in a bunch of places.  And I'm dead serious about

Yes, that I did fix as far as I'm aware. If the rules would've been
written down where when something was freed we would've had an easier
time figuring this out though. But they weren't so we missed it.

> Documentation/filesystems/porting being the right place; any development

Yes, agreed. I'll write a document for Christoph's next version.

I know that what you're saying is roughly that we shouldn't make the
same mistake as were done before but the fact that the old lifetime
rules weren't documented in any meaningful way and now we get grumbled
at in turn makes me grumble a bit. :) But overall point duly taken.

> tree of any filesystem (in-tree one or not) will have to go through the
> changes and figure out WTF to do with their existing code.  We are
> going to play whack-a-mole for at least several years as development
> branches get rebased and merged.

Let me write something up.

> 
> Incidentally, I'm going to add a (belated by 10 years) chunk in porting.rst
> re making sure that anything in superblock that might be needed by methods
> called in RCU mode should *not* be freed without an RCU delay...  Should've
> done that back in 3.12 merge window when RCU'd vfsmounts went in; as it
> is, today we have several filesystems with exact same kind of breakage.
> hfsplus and affs breakage had been there in 3.13 (missed those two), exfat
> and ntfs3 - introduced later, by initial merges of filesystems in question.
> Missed on review...

Cool, thanks for adding that.

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christian Brauner]

> > tree of any filesystem (in-tree one or not) will have to go through the
> > changes and figure out WTF to do with their existing code.  We are
> > going to play whack-a-mole for at least several years as development
> > branches get rebased and merged.
> 
> Let me write something up.

So here I've written two porting.rst patches that aim to reflect the
current state of things (They do _not_ reflect what's in Christoph's
series here as that'ss again pretty separate and will require additional
spelling out.).

I'm adding explanation for both the old and new logic fwiw. I hope to
upstream these docs soon so we all have something to point to.

From 200666901f53db74edf309d48e3c74fd275a822a Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Fri, 15 Sep 2023 16:01:02 +0200
Subject: [PATCH 1/2] porting: document new block device opening order

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 Documentation/filesystems/porting.rst | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
index deac4e973ddc..f436b64b77bf 100644
--- a/Documentation/filesystems/porting.rst
+++ b/Documentation/filesystems/porting.rst
@@ -949,3 +949,27 @@ mmap_lock held.  All in-tree users have been audited and do not seem to
 depend on the mmap_lock being held, but out of tree users should verify
 for themselves.  If they do need it, they can return VM_FAULT_RETRY to
 be called with the mmap_lock held.
+
+---
+
+**mandatory**
+
+The order of opening block devices and matching or creating superblocks has
+changed.
+
+The old logic opened block devices first and then tried to find a
+suitable superblock to reuse based on the block device pointer.
+
+The new logic finds or creates a superblock first, opening block devices
+afterwards. Since opening block devices cannot happen under s_umount because of
+lock ordering requirements s_umount is now dropped while opening block
+devices and reacquired before calling fill_super().
+
+In the old logic concurrent mounters would find the superblock on the list of
+active superblock for the filesystem type. Since the first opener of the block
+device would hold s_umount they would wait until the superblock became either
+born or died prematurely due to initialization failure.
+
+Since the new logic drops s_umount concurrent mounters could grab s_umount and
+would spin. Instead they are now made to wait using an explicit wait-wake
+mechanism without having to hold s_umount.
-- 
2.34.1

From 1f09898322b4402219d8d3219d399c9e56a76bae Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Fri, 15 Sep 2023 16:01:40 +0200
Subject: [PATCH 2/2] porting: document superblock as block device holder

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 Documentation/filesystems/porting.rst | 79 +++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
index f436b64b77bf..fefefaf289b4 100644
--- a/Documentation/filesystems/porting.rst
+++ b/Documentation/filesystems/porting.rst
@@ -973,3 +973,82 @@ born or died prematurely due to initialization failure.
 Since the new logic drops s_umount concurrent mounters could grab s_umount and
 would spin. Instead they are now made to wait using an explicit wait-wake
 mechanism without having to hold s_umount.
+
+---
+
+**mandatory**
+
+The holder of a block device is now the superblock.
+
+The holder of a block device used to be the file_system_type which wasn't
+particularly useful. It wasn't possible to go from block device to owning
+superblock without matching on the device pointer stored in the superblock.
+This mechanism would only work for a single device so the block layer couldn't
+find the owning superblock associated with additional devices.
+
+In the old mechanism reusing or creating a superblock for racing mount(2) and
+umount(2) relied on the file_system_type as the holder. This was severly
+underdocumented however:
+
+(1) If the concurrent mount(2) managed to grab an active reference before the
+    umount(2) dropped the last active reference in deactivate_locked_super()
+    the mounter would simply reuse the existing superblock.
+
+(2) If the mounter came after deactivate_locked_super() but before
+    the superblock had been removed from the list of superblocks of the
+    filesystem type the mounter would wait until the superblock was shutdown
+    and allocated a new superblock.
+
+(3) If the mounter came after deactivate_locked_super() and after
+    the superblock had been removed from the list of superblocks of the
+    filesystem type the mounter would allocate a new superblock.
+
+Because the holder of the block device was the filesystem type any concurrent
+mounter could open the block device without risking seeing EBUSY because the
+block device was still in use.
+
+Making the superblock the owner of the block device changes this as the holder
+is now a unique superblock and not shared among all superblocks of the
+filesystem type. So a concurrent mounter in (2) could suddenly see EBUSY when
+trying to open a block device whose holder was a different superblock.
+
+The new logic thus waits until the superblock and the devices are shutdown in
+->kill_sb(). Removal of the superblock from the list of superblocks of the
+filesystem type is now moved to a later point when the devices are closed:
+
+(1) Any concurrent mounter managing to grab an active reference on an existing
+    superblock is made to wait until the superblock is either ready or until
+    the superblock and all devices are shutdown in ->kill_sb().
+
+(2) If the mounter came after deactivate_locked_super() but before
+    the superblock had been removed from the list of superblocks of the
+    filesystem type the mounter is made to wait until the superblock and the
+    devices are shut down in ->kill_sb() and the superblock is removed from the
+    list of superblocks of the filesystem type.
+
+(3) This case is now collapsed into (2) as the superblock is left on the list
+    of superblocks of the filesystem type until all devices are shutdown in
+    ->kill_sb().
+
+As this is a VFS level change it has no practical consequences for filesystems
+other than that all of them must use one of the provided kill_litter_super(),
+kill_anon_super(), or kill_block_super() helpers.
+
+Filesystems that reuse superblocks based on non-static keys such as
+sb->s_fs_info must ensure that these keys remain valid across kill_*_super()
+calls. The expected pattern is::
+
+	static struct file_system_type some_fs_type = {
+		.name 		= "somefs",
+		.kill_sb 	= some_fs_kill_sb,
+	};
+
+	static void some_fs_kill_sb(struct super_block *sb)
+	{
+		struct some_fs_info *info = sb->s_fs_info;
+
+		kill_*_super(sb);
+		kfree(info);
+	}
+
+It's best practice to never deviate from this pattern.
-- 
2.34.1

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Al Viro]

On Fri, Sep 15, 2023 at 04:12:07PM +0200, Christian Brauner wrote:
> +	static void some_fs_kill_sb(struct super_block *sb)
> +	{
> +		struct some_fs_info *info = sb->s_fs_info;
> +
> +		kill_*_super(sb);
> +		kfree(info);
> +	}
> +
> +It's best practice to never deviate from this pattern.

The last part is flat-out incorrect.  If e.g. fatfs or cifs ever switches
to that pattern, you'll get UAF - they need freeing of ->s_fs_info
of anything that ever had been mounted done with RCU delay; moreover,
unload_nls() in fatfs needs to be behind the same.

Lifetime rules for fs-private parts of superblock are really private to
filesystem; their use by sget/sget_fc callbacks might impose restrictions
on those, but that again is none of the VFS business.

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Al Viro]

On Fri, Sep 15, 2023 at 03:28:14PM +0100, Al Viro wrote:
> On Fri, Sep 15, 2023 at 04:12:07PM +0200, Christian Brauner wrote:
> > +	static void some_fs_kill_sb(struct super_block *sb)
> > +	{
> > +		struct some_fs_info *info = sb->s_fs_info;
> > +
> > +		kill_*_super(sb);
> > +		kfree(info);
> > +	}
> > +
> > +It's best practice to never deviate from this pattern.
> 
> The last part is flat-out incorrect.  If e.g. fatfs or cifs ever switches
> to that pattern, you'll get UAF - they need freeing of ->s_fs_info
> of anything that ever had been mounted done with RCU delay; moreover,
> unload_nls() in fatfs needs to be behind the same.
> 
> Lifetime rules for fs-private parts of superblock are really private to
> filesystem; their use by sget/sget_fc callbacks might impose restrictions
> on those, but that again is none of the VFS business.

PS: and no, we don't want to impose such RCU delay on every filesystem
out there; what's more, there's nothing to prohibit e.g. having ->s_fs_info
pointing to a refcounted fs-private object (possibly shared by various
superblocks), so freeing might very well be "drop the reference and destroy
if refcount has reached 0".

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christian Brauner]

> Lifetime rules for fs-private parts of superblock are really private to

Fine, I'll drop that. It's still correct that a filesystem needs to take
care when it frees sb->s_fs_info. See the RCU fun you just encountered.

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christoph Hellwig]

On Thu, Sep 14, 2023 at 06:38:43AM +0100, Al Viro wrote:
> It's worse - look at the rationale for 2c18a63b760a "super: wait until
> we passed kill super".  Basically, "don't remove from the lists
> until after block device closing".  IOW, we have

As of this stage we don't even touch anything related to block devices..

> That was *NOT* what a recent rework had done.  Block device closing had never
> been inside ->put_super() - at no point since that (closing, that is) had been
> introduced back in 0.97 ;-)  ->put_super() predates it (0.95c+).
> 
> The race is real, but the cause is not some kind of move of blkdev_put().
> Your 2ea6f68932f7 "fs: use the super_block as holder when mounting file
> systems" is where it actually came from.
> 
> Christoph, could you explain what the hell do we need that for?  It does
> create the race in question and AFAICS 2c18a63b760a (and followups trying
> to plug holes in it) had been nothing but headache.

Because it allows us to actually get from the bdev to the holder directly,
someting we've badly neeed for a while.

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christoph Hellwig]

On Thu, Sep 14, 2023 at 12:27:12AM +0100, Al Viro wrote:
> On Wed, Sep 13, 2023 at 08:09:57AM -0300, Christoph Hellwig wrote:
> > Releasing an anon dev_t is a very common thing when freeing a
> > super_block, as that's done for basically any not block based file
> > system (modulo the odd mtd special case).  So instead of requiring
> > a special ->kill_sb helper and a lot of boilerplate in more complicated
> > file systems, just release the anon dev_t in deactivate_locked_super if
> > the super_block was using one.
> > 
> > As the freeing is done after the main call to kill_super_notify, this
> > removes the need for having two slightly different call sites for it.
> 
> Huh?  At this stage in your series freeing is still in ->kill_sb()
> instances, after the calls of kill_anon_super() you've turned into
> the calls of generic_shutdown_super().

The above refers to freeing the anon dev_t, which at this stage is done
right after the kill_super_notify in generic_shutdown_super.

> You do split it off into a separate method later in the series, but
> at this point you are reopening the same UAF that had been dealt with
> in dc3216b14160 "super: ensure valid info".

How?

Old sequence before his patch:

	deactivate_locked_super()
	  -> kill_anon_super()
	    -> generic_shutdown_super()
	    -> kill_super_notify()
	    -> free_anon_bdev()
	  -> kill_super_notify()

New sequence with this patch:

	deactivate_locked_super()
	  -> generic_shutdown_super()
	    -> kill_super_notify()
	    -> free_anon_bdev()

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Al Viro]

On Tue, Sep 26, 2023 at 11:38:34AM +0200, Christoph Hellwig wrote:

> How?
> 
> Old sequence before his patch:
> 
> 	deactivate_locked_super()
> 	  -> kill_anon_super()
> 	    -> generic_shutdown_super()
> 	    -> kill_super_notify()
> 	    -> free_anon_bdev()
> 	  -> kill_super_notify()
> 
> New sequence with this patch:
> 
> 	deactivate_locked_super()
> 	  -> generic_shutdown_super()
> 	    -> kill_super_notify()
> 	    -> free_anon_bdev()
> 

Before your patch: foo_kill_super() calls kill_anon_super(),
which calls kill_super_notify(), which removes the sucker from
the list, then frees ->s_fs_info.  After your patch:
removal from the lists happens via the call of kill_super_notify()
*after* both of your methods had been called, while freeing
->s_fs_info happens from the method call.  IOW, you've restored
the situation prior to "super: ensure valid info".  The whole
point of that commit had been to make sure that we have nothing
in the lists with ->s_fs_info pointing to a freed object.

It's not about free_anon_bdev(); that part is fine - it's the
"we can drop the weird second call site of kill_super_notify()"
thing that is broken.

Al, still slogging through the rcu pathwalk races in the methods...
The latest catch: nfs_set_verifier() can get called on a dentry
that had just been seen to have positive parent, but is not
pinned down.
	grab ->d_lock; OK, we know that dentry won't get freed under us
	fetch ->d_parent->d_inode
	pass that to nfs_verify_change_attribute()
... which assumes that inode it's been given is not NULL.  Normally it
would've been - ->d_lock stabilizes ->d_parent, and negative dentries
obviously have no children.  Except that we might've been just hit
by dentry_kill() due to eviction on memory pressure, got ->d_lock
right after that and proceeded to play with ->d_parent, just as
that parent is going through dentry_kill() from the same eviction on
memory pressure...  If it gets to dentry_unlink_inode() before we get to
fetching ->d_parent->d_inode, nfs_verify_change_attribute(NULL, whatever)
is going to oops...

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Al Viro]

On Tue, Sep 26, 2023 at 10:25:15PM +0100, Al Viro wrote:

> Before your patch: foo_kill_super() calls kill_anon_super(),
> which calls kill_super_notify(), which removes the sucker from
> the list, then frees ->s_fs_info.  After your patch:
> removal from the lists happens via the call of kill_super_notify()
> *after* both of your methods had been called, while freeing
> ->s_fs_info happens from the method call.  IOW, you've restored
> the situation prior to "super: ensure valid info".  The whole
> point of that commit had been to make sure that we have nothing
> in the lists with ->s_fs_info pointing to a freed object.

More detailed example: take a look at NFS.  We have ->get_tree() there
call sget_fc() with nfs_compare_super() as possible 'test' callback.
It does look at ->s_fs_info of the superblocks found on the list
of instances for fs type in question.  Moreover, it proceeds to
call nfs_compare_mount_options(), which chases pointers from that
(at the very least fetch ->client in nfs_server instance ->s_fs_info
points to and dereferences that).

We really, really do not want nfs_free_server() happen while the
superblock is visible in the instances list.  Now, in your tree
nfs_free_sb() call nfs_free_server().  *Without* having called
kill_super_notify() first - you do that only after the call of
->free_sb().

So with this series applied we have UAF on race between mount and
umount.  For NFS.  No block devices involved.

Old logics had been "after generic_shutdown_super() the private
parts of superblock belong to filesystem alone; they might be
accessed by methods called from RCU pathwalk, but that's it".

I still don't see any clear rules for the new one.  And the more
I'm looking, the more sceptical I get about the approach you've
taken, TBH...

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christoph Hellwig]

On Tue, Sep 26, 2023 at 10:25:15PM +0100, Al Viro wrote:
> Before your patch: foo_kill_super() calls kill_anon_super(),
> which calls kill_super_notify(), which removes the sucker from
> the list, then frees ->s_fs_info.  After your patch:
> removal from the lists happens via the call of kill_super_notify()
> *after* both of your methods had been called, while freeing
> ->s_fs_info happens from the method call.  IOW, you've restored
> the situation prior to "super: ensure valid info".  The whole
> point of that commit had been to make sure that we have nothing
> in the lists with ->s_fs_info pointing to a freed object.
> 
> It's not about free_anon_bdev(); that part is fine - it's the
> "we can drop the weird second call site of kill_super_notify()"
> thing that is broken.

The point has been to only release the anon dev_t after
kill_super_notify, to prevent two of them beeing reused.

Which we do as the free_anon_bdev is done directly in
deactivate_locked_super.  The new ->free_sb for non-block file systems
frees resources, but none of them matter for sget.

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Al Viro]

On Mon, Oct 02, 2023 at 08:46:46AM +0200, Christoph Hellwig wrote:
> On Tue, Sep 26, 2023 at 10:25:15PM +0100, Al Viro wrote:
> > Before your patch: foo_kill_super() calls kill_anon_super(),
> > which calls kill_super_notify(), which removes the sucker from
> > the list, then frees ->s_fs_info.  After your patch:
> > removal from the lists happens via the call of kill_super_notify()
> > *after* both of your methods had been called, while freeing
> > ->s_fs_info happens from the method call.  IOW, you've restored
> > the situation prior to "super: ensure valid info".  The whole
> > point of that commit had been to make sure that we have nothing
> > in the lists with ->s_fs_info pointing to a freed object.
> > 
> > It's not about free_anon_bdev(); that part is fine - it's the
> > "we can drop the weird second call site of kill_super_notify()"
> > thing that is broken.
> 
> The point has been to only release the anon dev_t after
> kill_super_notify, to prevent two of them beeing reused.
> 
> Which we do as the free_anon_bdev is done directly in
> deactivate_locked_super.  The new ->free_sb for non-block file systems
> frees resources, but none of them matter for sget.

We keep talking past each other...  Let me try again:
at the tip of your branch you have

static struct file_system_type ubifs_fs_type = {
        .name    = "ubifs",
	.owner   = THIS_MODULE,
	.mount   = ubifs_mount,
	.free_sb = ubifs_free_sb,
};

static void ubifs_free_sb(struct super_block *s)
{
        kfree(s->s_fs_info);
}

static struct dentry *ubifs_mount(struct file_system_type *fs_type, int flags,
                        const char *name, void *data)
{
	...
        sb = sget(fs_type, sb_test, sb_set, flags, c);
	...
}

static int sb_test(struct super_block *sb, void *data)
{
        struct ubifs_info *c1 = data;
        struct ubifs_info *c = sb->s_fs_info;

        return c->vi.cdev == c1->vi.cdev;
}

See the problem?  Mainline has

static void kill_ubifs_super(struct super_block *s)
{
        struct ubifs_info *c = s->s_fs_info;
        kill_anon_super(s);
        kfree(c);
}
and
void kill_anon_super(struct super_block *sb)
{
        dev_t dev = sb->s_dev;
        generic_shutdown_super(sb);
        kill_super_notify(sb);
        free_anon_bdev(dev);
}

That removes the superblock from the list of instances before its
->s_fs_info is freed.  In your branch removal happens here:

        if (fs->shutdown_sb)
                fs->shutdown_sb(s);
        generic_shutdown_super(s);
        if (fs->free_sb)
                fs->free_sb(s);

        kill_super_notify(s);

That comes *after* ubifs_free_sb() has freed ->s_fs_info.  And there's
nothing to stop ubifs_mount() (on a completely unrelated device) to get
called right at that moment.  Doing the sget() call quoted above.  Now,
in sget() we have
                hlist_for_each_entry(old, &type->fs_supers, s_instances) {
                        if (!test(old, data))
and that will hit sb_test(old, data), with old being a superblock still
in ->fs_supers, but with ->s_fs_info already freed.  So in sb_test()
we have c equal to old->s_fs_info and
        return c->vi.cdev == c1->vi.cdev;
is a bloody use after free.

Here we are unlikely to get fucked over - it's a plain fetch from freed
object.  If you look at e.g. nfs, you'll see a lot more than that -
pointer chasing from freed (and possibly reused) object.  The only
difference is that there you have sget_fc() instead of sget() - same
loop anyway.

The bottom line: in the form it is posted, your series reintroduces the
class of UAF that had been added by taking removal from the instances
list out of generic_shutdown_super() and then papered over by adding
that kill_super_notify() into kill_anon_super().

And frankly, I believe that the root cause is the insistence that
list removal should happen after generic_shutdown_super().  Sure, you
want the superblock to serve as bdev holder, which leads to fun
with -EBUSY if mount comes while umount still hadn't closed the
device.  I suspect that it would make a lot more sense to
introduce an intermediate state - "held, but will be released
in a short while".  You already have something similar, but
only for the entire disk ->bd_claiming stuff.

Add a new primitive (will_release_bdev()), so that attempts to
claim the sucker will wait until it gets released instead of
failing with -EBUSY.  And do *that* before generic_shutdown_super()
when unmounting something that is block-based.  Allows to bring
the list removal back where it used to be, no UAF at all...

IMO that direction is a lot more promising.

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Christian Brauner]

> list removal should happen after generic_shutdown_super().  Sure, you
> want the superblock to serve as bdev holder, which leads to fun
> with -EBUSY if mount comes while umount still hadn't closed the
> device.  I suspect that it would make a lot more sense to
> introduce an intermediate state - "held, but will be released
> in a short while".  You already have something similar, but
> only for the entire disk ->bd_claiming stuff.
> 
> Add a new primitive (will_release_bdev()), so that attempts to
> claim the sucker will wait until it gets released instead of
> failing with -EBUSY.  And do *that* before generic_shutdown_super()
> when unmounting something that is block-based.  Allows to bring
> the list removal back where it used to be, no UAF at all...

This is essentially equivalent to what is done right now. Only that this
would then happen in the block layer. I'm not sure it would buy us that
much. In all likelyhood we just get a range of other issues to fix.

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

[Al Viro]

On Tue, Oct 10, 2023 at 10:44:09AM +0200, Christian Brauner wrote:
> > list removal should happen after generic_shutdown_super().  Sure, you
> > want the superblock to serve as bdev holder, which leads to fun
> > with -EBUSY if mount comes while umount still hadn't closed the
> > device.  I suspect that it would make a lot more sense to
> > introduce an intermediate state - "held, but will be released
> > in a short while".  You already have something similar, but
> > only for the entire disk ->bd_claiming stuff.
> > 
> > Add a new primitive (will_release_bdev()), so that attempts to
> > claim the sucker will wait until it gets released instead of
> > failing with -EBUSY.  And do *that* before generic_shutdown_super()
> > when unmounting something that is block-based.  Allows to bring
> > the list removal back where it used to be, no UAF at all...
> 
> This is essentially equivalent to what is done right now. Only that this
> would then happen in the block layer. I'm not sure it would buy us that
> much. In all likelyhood we just get a range of other issues to fix.

The difference is, we separate the "close the block device" (which
can't be done until we stopped generating any IO on it, obviously)
from "tell anyone who wants to claim the sucker that we are going
to release it and they just need to wait".  That can be done before
generic_shutdown_super(), or from it (e.g. from ->put_super()),
untangling the ordering mess.

[PATCH 04/19] NFS: remove the s_dev field from struct nfs_server

[Christoph Hellwig]

Don't duplicate the dev_t in the nfs_server structure given that it can
be trivially retrieved from the super_block.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/client.c           |  2 +-
 fs/nfs/nfs4proc.c         |  8 ++++----
 fs/nfs/nfs4trace.h        |  6 +++---
 fs/nfs/nfs4xdr.c          |  2 +-
 fs/nfs/super.c            | 10 +++-------
 include/linux/nfs_fs_sb.h |  1 -
 6 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 44eca51b28085d..039fd67ac17c82 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1337,7 +1337,7 @@ static int nfs_volume_list_show(struct seq_file *m, void *v)
 	clp = server->nfs_client;
 
 	snprintf(dev, sizeof(dev), "%u:%u",
-		 MAJOR(server->s_dev), MINOR(server->s_dev));
+		 MAJOR(server->super->s_dev), MINOR(server->super->s_dev));
 
 	snprintf(fsid, sizeof(fsid), "%llx:%llx",
 		 (unsigned long long) server->fsid.major,
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 794343790ea8bb..4d002cc514983c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6763,7 +6763,7 @@ static int _nfs4_proc_getlk(struct nfs4_state *state, int cmd, struct file_lock
 		goto out;
 	lsp = request->fl_u.nfs4_fl.owner;
 	arg.lock_owner.id = lsp->ls_seqid.owner_id;
-	arg.lock_owner.s_dev = server->s_dev;
+	arg.lock_owner.s_dev = server->super->s_dev;
 	status = nfs4_call_sync(server->client, server, &msg, &arg.seq_args, &res.seq_res, 1);
 	switch (status) {
 		case 0:
@@ -7088,7 +7088,7 @@ static struct nfs4_lockdata *nfs4_alloc_lockdata(struct file_lock *fl,
 		goto out_free_seqid;
 	p->arg.lock_owner.clientid = server->nfs_client->cl_clientid;
 	p->arg.lock_owner.id = lsp->ls_seqid.owner_id;
-	p->arg.lock_owner.s_dev = server->s_dev;
+	p->arg.lock_owner.s_dev = server->super->s_dev;
 	p->res.lock_seqid = p->arg.lock_seqid;
 	p->lsp = lsp;
 	p->server = server;
@@ -7475,7 +7475,7 @@ nfs4_retry_setlk(struct nfs4_state *state, int cmd, struct file_lock *request)
 		.inode = state->inode,
 		.owner = { .clientid = clp->cl_clientid,
 			   .id = lsp->ls_seqid.owner_id,
-			   .s_dev = server->s_dev },
+			   .s_dev = server->super->s_dev },
 	};
 	int status;
 
@@ -7689,7 +7689,7 @@ nfs4_release_lockowner(struct nfs_server *server, struct nfs4_lock_state *lsp)
 	data->server = server;
 	data->args.lock_owner.clientid = server->nfs_client->cl_clientid;
 	data->args.lock_owner.id = lsp->ls_seqid.owner_id;
-	data->args.lock_owner.s_dev = server->s_dev;
+	data->args.lock_owner.s_dev = server->super->s_dev;
 
 	msg.rpc_argp = &data->args;
 	msg.rpc_resp = &data->res;
diff --git a/fs/nfs/nfs4trace.h b/fs/nfs/nfs4trace.h
index d27919d7241d38..13a602c675ddb2 100644
--- a/fs/nfs/nfs4trace.h
+++ b/fs/nfs/nfs4trace.h
@@ -945,7 +945,7 @@ TRACE_EVENT(nfs4_delegreturn_exit,
 		),
 
 		TP_fast_assign(
-			__entry->dev = res->server->s_dev;
+			__entry->dev = res->server->super->s_dev;
 			__entry->fhandle = nfs_fhandle_hash(args->fhandle);
 			__entry->error = error < 0 ? -error : 0;
 			__entry->stateid_seq =
@@ -1269,7 +1269,7 @@ DECLARE_EVENT_CLASS(nfs4_getattr_event,
 		),
 
 		TP_fast_assign(
-			__entry->dev = server->s_dev;
+			__entry->dev = server->super->s_dev;
 			__entry->valid = fattr->valid;
 			__entry->fhandle = nfs_fhandle_hash(fhandle);
 			__entry->fileid = (fattr->valid & NFS_ATTR_FATTR_FILEID) ? fattr->fileid : 0;
@@ -1966,7 +1966,7 @@ DECLARE_EVENT_CLASS(nfs4_deviceid_status,
 		),
 
 		TP_fast_assign(
-			__entry->dev = server->s_dev;
+			__entry->dev = server->super->s_dev;
 			__entry->status = status;
 			__assign_str(dstaddr, server->nfs_client->cl_hostname);
 			memcpy(__entry->deviceid, deviceid->data,
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index deec76cf5afeaf..9767c5e2ed1a9a 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1420,7 +1420,7 @@ static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_opena
 	p = xdr_encode_hyper(p, arg->clientid);
 	*p++ = cpu_to_be32(24);
 	p = xdr_encode_opaque_fixed(p, "open id:", 8);
-	*p++ = cpu_to_be32(arg->server->s_dev);
+	*p++ = cpu_to_be32(arg->server->super->s_dev);
 	*p++ = cpu_to_be32(arg->id.uniquifier);
 	xdr_encode_hyper(p, arg->id.create_time);
 }
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 29d6a55b9d400d..561221a87b02a6 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1130,13 +1130,9 @@ static int nfs_compare_mount_options(const struct super_block *s, const struct n
 static int nfs_set_super(struct super_block *s, struct fs_context *fc)
 {
 	struct nfs_server *server = fc->s_fs_info;
-	int ret;
 
 	s->s_d_op = server->nfs_client->rpc_ops->dentry_ops;
-	ret = set_anon_super(s, server);
-	if (ret == 0)
-		server->s_dev = s->s_dev;
-	return ret;
+	return set_anon_super(s, server);
 }
 
 static int nfs_compare_super_address(struct nfs_server *server1,
@@ -1292,8 +1288,8 @@ int nfs_get_tree_common(struct fs_context *fc)
 		nfs_free_server(server);
 		server = NULL;
 	} else {
-		error = super_setup_bdi_name(s, "%u:%u", MAJOR(server->s_dev),
-					     MINOR(server->s_dev));
+		error = super_setup_bdi_name(s, "%u:%u", MAJOR(s->s_dev),
+					     MINOR(s->s_dev));
 		if (error)
 			goto error_splat_super;
 		s->s_bdi->io_pages = server->rpages;
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 20eeba8b009df1..9c71af0e3516fa 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -190,7 +190,6 @@ struct nfs_server {
 	struct timespec64	time_delta;	/* smallest time granularity */
 	unsigned long		mount_time;	/* when this fs was mounted */
 	struct super_block	*super;		/* VFS super block */
-	dev_t			s_dev;		/* superblock dev numbers */
 	struct nfs_auth_info	auth_info;	/* parsed auth flavors */
 
 #ifdef CONFIG_NFS_FSCACHE
-- 
2.39.2

[PATCH 05/19] fs: assign an anon dev_t in common code

[Christoph Hellwig]

All super_blocks need to have a valid dev_t, and except for block
based file systems that tends to be an anonymouns dev_t.  Instead of
leaving that work to the file systems, assign the anonymous dev_t in
the core sget_fc and sget routines unless the file systems already
assigned on in the set callback.  Note that this now makes the
set callback optional as a lot of file systems don't need it any more.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/9p/vfs_super.c      |  2 +-
 fs/afs/super.c         | 12 +++-----
 fs/btrfs/super.c       |  6 ++--
 fs/ceph/super.c        |  7 +----
 fs/ecryptfs/main.c     |  2 +-
 fs/fuse/inode.c        |  2 +-
 fs/fuse/virtio_fs.c    |  2 +-
 fs/kernfs/mount.c      |  2 +-
 fs/nfs/super.c         |  2 +-
 fs/orangefs/super.c    |  2 +-
 fs/smb/client/cifsfs.c |  3 +-
 fs/super.c             | 68 +++++++++++++++++++++++++-----------------
 fs/ubifs/super.c       |  2 +-
 include/linux/fs.h     |  2 --
 14 files changed, 58 insertions(+), 56 deletions(-)

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 9e60eddf5179ed..e8b3641c98f886 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -40,7 +40,7 @@ static const struct super_operations v9fs_super_ops, v9fs_super_ops_dotl;
 static int v9fs_set_super(struct super_block *s, void *data)
 {
 	s->s_fs_info = data;
-	return set_anon_super(s, data);
+	return 0;
 }
 
 /**
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 754b9828233497..84b135ad3496b1 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -435,11 +435,6 @@ static int afs_dynroot_test_super(struct super_block *sb, struct fs_context *fc)
 		as->dyn_root);
 }
 
-static int afs_set_super(struct super_block *sb, struct fs_context *fc)
-{
-	return set_anon_super(sb, NULL);
-}
-
 /*
  * fill in the superblock
  */
@@ -574,9 +569,10 @@ static int afs_get_tree(struct fs_context *fc)
 	fc->s_fs_info = as;
 
 	/* allocate a deviceless superblock */
-	sb = sget_fc(fc,
-		     as->dyn_root ? afs_dynroot_test_super : afs_test_super,
-		     afs_set_super);
+	if (as->dyn_root)
+		sb = sget_fc(fc, afs_dynroot_test_super, NULL);
+	else
+		sb = sget_fc(fc, afs_test_super, NULL);
 	if (IS_ERR(sb)) {
 		ret = PTR_ERR(sb);
 		goto error;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 01b86bd4eae8dc..063b9aa313c227 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1350,10 +1350,8 @@ static int btrfs_test_super(struct super_block *s, void *data)
 
 static int btrfs_set_super(struct super_block *s, void *data)
 {
-	int err = set_anon_super(s, data);
-	if (!err)
-		s->s_fs_info = data;
-	return err;
+	s->s_fs_info = data;
+	return 0;
 }
 
 /*
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 7feef0b35b97b5..cbeaab8c21d8e6 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1191,7 +1191,6 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc,
 static int ceph_set_super(struct super_block *s, struct fs_context *fc)
 {
 	struct ceph_fs_client *fsc = s->s_fs_info;
-	int ret;
 
 	dout("set_super %p\n", s);
 
@@ -1211,11 +1210,7 @@ static int ceph_set_super(struct super_block *s, struct fs_context *fc)
 	s->s_flags |= SB_NODIRATIME | SB_NOATIME;
 
 	ceph_fscrypt_set_ops(s);
-
-	ret = set_anon_super_fc(s, fc);
-	if (ret != 0)
-		fsc->sb = NULL;
-	return ret;
+	return 0;
 }
 
 /*
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index d99b2311759166..3ed91537a3991a 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -505,7 +505,7 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
 	}
 	mount_crypt_stat = &sbi->mount_crypt_stat;
 
-	s = sget(fs_type, NULL, set_anon_super, flags, NULL);
+	s = sget(fs_type, NULL, NULL, flags, NULL);
 	if (IS_ERR(s)) {
 		rc = PTR_ERR(s);
 		goto out;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 42523edb32fd53..5731003b56a9c9 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1511,7 +1511,7 @@ static int fuse_get_tree_submount(struct fs_context *fsc)
 
 	fm->fc = fuse_conn_get(fc);
 	fsc->s_fs_info = fm;
-	sb = sget_fc(fsc, NULL, set_anon_super_fc);
+	sb = sget_fc(fsc, NULL, NULL);
 	if (fsc->s_fs_info)
 		fuse_mount_destroy(fm);
 	if (IS_ERR(sb))
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 0a0d593e5a9c79..a52957df956394 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1454,7 +1454,7 @@ static int virtio_fs_get_tree(struct fs_context *fsc)
 				    virtqueue_size - FUSE_HEADER_OVERHEAD);
 
 	fsc->s_fs_info = fm;
-	sb = sget_fc(fsc, virtio_fs_test_super, set_anon_super_fc);
+	sb = sget_fc(fsc, virtio_fs_test_super, NULL);
 	if (fsc->s_fs_info)
 		fuse_mount_destroy(fm);
 	if (IS_ERR(sb))
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index 772d059d4054ec..d6d3cba669dbdd 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -300,7 +300,7 @@ static int kernfs_set_super(struct super_block *sb, struct fs_context *fc)
 	struct kernfs_fs_context *kfc = fc->fs_private;
 
 	kfc->ns_tag = NULL;
-	return set_anon_super_fc(sb, fc);
+	return 0;
 }
 
 /**
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 561221a87b02a6..89131e855e1393 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1132,7 +1132,7 @@ static int nfs_set_super(struct super_block *s, struct fs_context *fc)
 	struct nfs_server *server = fc->s_fs_info;
 
 	s->s_d_op = server->nfs_client->rpc_ops->dentry_ops;
-	return set_anon_super(s, server);
+	return 0;
 }
 
 static int nfs_compare_super_address(struct nfs_server *server1,
diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index 42cb3e9b1effee..bf3a834ad15033 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -514,7 +514,7 @@ struct dentry *orangefs_mount(struct file_system_type *fst,
 		goto free_op;
 	}
 
-	sb = sget(fst, NULL, set_anon_super, flags, NULL);
+	sb = sget(fst, NULL, NULL, flags, NULL);
 
 	if (IS_ERR(sb)) {
 		d = ERR_CAST(sb);
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index fb792acffeca37..5700f1065af756 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -884,8 +884,9 @@ cifs_get_root(struct smb3_fs_context *ctx, struct super_block *sb)
 static int cifs_set_super(struct super_block *sb, void *data)
 {
 	struct cifs_mnt_data *mnt_data = data;
+
 	sb->s_fs_info = mnt_data->cifs_sb;
-	return set_anon_super(sb, NULL);
+	return 0;
 }
 
 struct dentry *
diff --git a/fs/super.c b/fs/super.c
index bbe55f0651cca4..5c685b4944c2d6 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -787,7 +787,7 @@ struct super_block *sget_fc(struct fs_context *fc,
 	struct super_block *s = NULL;
 	struct super_block *old;
 	struct user_namespace *user_ns = fc->global ? &init_user_ns : fc->user_ns;
-	int err;
+	int err = 0;
 
 retry:
 	spin_lock(&sb_lock);
@@ -806,14 +806,26 @@ struct super_block *sget_fc(struct fs_context *fc,
 	}
 
 	s->s_fs_info = fc->s_fs_info;
-	err = set(s, fc);
-	if (err) {
-		s->s_fs_info = NULL;
-		spin_unlock(&sb_lock);
-		destroy_unused_super(s);
-		return ERR_PTR(err);
+	if (set) {
+		err = set(s, fc);
+		if (err) {
+			s->s_fs_info = NULL;
+			goto unlock_and_destroy;
+		}
 	}
 	fc->s_fs_info = NULL;
+
+	if (!s->s_dev) {
+		/*
+		 * If the file system didn't set a s_dev (which is usually only
+		 * done for block based file systems), set an anonymous dev_t
+		 * here now so that we always have a valid ->s_dev.
+		 */
+		err = get_anon_bdev(&s->s_dev);
+		if (err)
+			goto unlock_and_destroy;
+	}
+
 	s->s_type = fc->fs_type;
 	s->s_iflags |= fc->s_iflags;
 	strscpy(s->s_id, s->s_type->name, sizeof(s->s_id));
@@ -843,6 +855,10 @@ struct super_block *sget_fc(struct fs_context *fc,
 		goto retry;
 	destroy_unused_super(s);
 	return old;
+unlock_and_destroy:
+	spin_unlock(&sb_lock);
+	destroy_unused_super(s);
+	return ERR_PTR(err);
 }
 EXPORT_SYMBOL(sget_fc);
 
@@ -897,12 +913,18 @@ struct super_block *sget(struct file_system_type *type,
 		goto retry;
 	}
 
-	err = set(s, data);
-	if (err) {
-		spin_unlock(&sb_lock);
-		destroy_unused_super(s);
-		return ERR_PTR(err);
+	if (set) {
+		err = set(s, data);
+		if (err)
+			goto unlock_and_destroy;
 	}
+
+	if (!s->s_dev) {
+		err = get_anon_bdev(&s->s_dev);
+		if (err)
+			goto unlock_and_destroy;
+	}
+
 	s->s_type = type;
 	strscpy(s->s_id, type->name, sizeof(s->s_id));
 	list_add_tail(&s->s_list, &super_blocks);
@@ -911,6 +933,10 @@ struct super_block *sget(struct file_system_type *type,
 	get_filesystem(type);
 	register_shrinker_prepared(&s->s_shrink);
 	return s;
+unlock_and_destroy:
+	spin_unlock(&sb_lock);
+	destroy_unused_super(s);
+	return ERR_PTR(err);
 }
 EXPORT_SYMBOL(sget);
 
@@ -1288,12 +1314,6 @@ void free_anon_bdev(dev_t dev)
 }
 EXPORT_SYMBOL(free_anon_bdev);
 
-int set_anon_super(struct super_block *s, void *data)
-{
-	return get_anon_bdev(&s->s_dev);
-}
-EXPORT_SYMBOL(set_anon_super);
-
 void kill_litter_super(struct super_block *sb)
 {
 	if (sb->s_root)
@@ -1302,12 +1322,6 @@ void kill_litter_super(struct super_block *sb)
 }
 EXPORT_SYMBOL(kill_litter_super);
 
-int set_anon_super_fc(struct super_block *sb, struct fs_context *fc)
-{
-	return set_anon_super(sb, NULL);
-}
-EXPORT_SYMBOL(set_anon_super_fc);
-
 static int test_keyed_super(struct super_block *sb, struct fs_context *fc)
 {
 	return sb->s_fs_info == fc->s_fs_info;
@@ -1326,7 +1340,7 @@ static int vfs_get_super(struct fs_context *fc,
 	struct super_block *sb;
 	int err;
 
-	sb = sget_fc(fc, test, set_anon_super_fc);
+	sb = sget_fc(fc, test, NULL);
 	if (IS_ERR(sb))
 		return PTR_ERR(sb);
 
@@ -1657,7 +1671,7 @@ struct dentry *mount_nodev(struct file_system_type *fs_type,
 	int (*fill_super)(struct super_block *, void *, int))
 {
 	int error;
-	struct super_block *s = sget(fs_type, NULL, set_anon_super, flags, NULL);
+	struct super_block *s = sget(fs_type, NULL, NULL, flags, NULL);
 
 	if (IS_ERR(s))
 		return ERR_CAST(s);
@@ -1709,7 +1723,7 @@ struct dentry *mount_single(struct file_system_type *fs_type,
 	struct super_block *s;
 	int error;
 
-	s = sget(fs_type, compare_single, set_anon_super, flags, NULL);
+	s = sget(fs_type, compare_single, NULL, flags, NULL);
 	if (IS_ERR(s))
 		return ERR_CAST(s);
 	if (!s->s_root) {
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 6527175591a729..b9d1ab63e3c87e 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2273,7 +2273,7 @@ static int sb_test(struct super_block *sb, void *data)
 static int sb_set(struct super_block *sb, void *data)
 {
 	sb->s_fs_info = data;
-	return set_anon_super(sb, NULL);
+	return 0;
 }
 
 static struct dentry *ubifs_mount(struct file_system_type *fs_type, int flags,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 129b8c0c83960b..31b6b235b36efa 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2385,8 +2385,6 @@ void kill_block_super(struct super_block *sb);
 void kill_litter_super(struct super_block *sb);
 void deactivate_super(struct super_block *sb);
 void deactivate_locked_super(struct super_block *sb);
-int set_anon_super(struct super_block *s, void *data);
-int set_anon_super_fc(struct super_block *s, struct fs_context *fc);
 int get_anon_bdev(dev_t *);
 void free_anon_bdev(dev_t);
 struct super_block *sget_fc(struct fs_context *fc,
-- 
2.39.2

Re: [PATCH 05/19] fs: assign an anon dev_t in common code

[Al Viro]

On Wed, Sep 13, 2023 at 08:09:59AM -0300, Christoph Hellwig wrote:

> diff --git a/fs/super.c b/fs/super.c
> index bbe55f0651cca4..5c685b4944c2d6 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -787,7 +787,7 @@ struct super_block *sget_fc(struct fs_context *fc,
>  	struct super_block *s = NULL;
>  	struct super_block *old;
>  	struct user_namespace *user_ns = fc->global ? &init_user_ns : fc->user_ns;
> -	int err;
> +	int err = 0;
>  
>  retry:
>  	spin_lock(&sb_lock);
> @@ -806,14 +806,26 @@ struct super_block *sget_fc(struct fs_context *fc,
>  	}
>  
>  	s->s_fs_info = fc->s_fs_info;
> -	err = set(s, fc);
> -	if (err) {
> -		s->s_fs_info = NULL;
> -		spin_unlock(&sb_lock);
> -		destroy_unused_super(s);
> -		return ERR_PTR(err);
> +	if (set) {
> +		err = set(s, fc);
> +		if (err) {
> +			s->s_fs_info = NULL;

Pointless (as the original had been); destroy_unused_super() doesn't
even look at ->s_fs_info.

> +			goto unlock_and_destroy;
> +		}
>  	}
>  	fc->s_fs_info = NULL;

Here we are transferring the ownership from fc to superblock; it used to sit
right next to insertion into lists and all failure exits past that point must
go through deactivate_locked_super(), so ->kill_sb() would be called on those
and it would take care of s->s_fs_info.  However, your variant has that
ownership transfer done at the point before get_anon_bdev(), and that got
you a new failure exit where you are still calling destroy_unused_super():

> +	if (!s->s_dev) {
> +		/*
> +		 * If the file system didn't set a s_dev (which is usually only
> +		 * done for block based file systems), set an anonymous dev_t
> +		 * here now so that we always have a valid ->s_dev.
> +		 */
> +		err = get_anon_bdev(&s->s_dev);
> +		if (err)
> +			goto unlock_and_destroy;

This.  And that's a leak - fc has no reference left in it, and your
unlock_and_destroy won't even look at what's in ->s_fs_info, let alone know
what to do with it.

IOW, clearing fc->s_fs_info should've been done after that chunk.

And looking at the change in sget(),

> +	if (set) {
> +		err = set(s, data);
> +		if (err)
> +			goto unlock_and_destroy;
>  	}
> +
> +	if (!s->s_dev) {
> +		err = get_anon_bdev(&s->s_dev);
> +		if (err)
> +			goto unlock_and_destroy;
> +	}

I'd rather expressed it (both there and in sget_fc() as well) as
	if (set)
		err = set(s, data);
	if (!err && !s->s_dev)
		err = get_anon_bdev(&s->s_dev);
	if (err)
		goto unlock_and_destroy;

That's really what your transformation does - you are lifting the
calls of get_anon_bdev() (in guise of set_anon_super()) from the
tails of 'set' callbacks into the caller, making them conditional
upon the lack of other errors from 'set' and upon the ->s_dev left
zero and allow NULL for the case when that was all that had been
there.

The only place where you do something different is this:

> @@ -1191,7 +1191,6 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc,
>  static int ceph_set_super(struct super_block *s, struct fs_context *fc)
>  {
>  	struct ceph_fs_client *fsc = s->s_fs_info;
> -	int ret;
>  
>  	dout("set_super %p\n", s);
>  
> @@ -1211,11 +1210,7 @@ static int ceph_set_super(struct super_block *s, struct fs_context *fc)
>  	s->s_flags |= SB_NODIRATIME | SB_NOATIME;
>  
>  	ceph_fscrypt_set_ops(s);
> -
> -	ret = set_anon_super_fc(s, fc);
> -	if (ret != 0)
> -		fsc->sb = NULL;
> -	return ret;
> +	return 0;

fsc->sb = NULL has disappeared here; it *is* OK (the caller won't look at
fsc->sb after failed sget_fc()), but that's worth a mention somewhere.
A separate commit removing that clearing fsc->sb in ceph_set_super(),
perhaps?

[PATCH 06/19] qibfs: use simple_release_fs

[Christoph Hellwig]

qibfs currently has convoluted code to allow registering HCAs while qibfs
is not mounted and vice versa.  Switch to using simple_release_fs every
time an entry is added to pin the fs instance and remove all the boiler
plate code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/infiniband/hw/qib/qib.h      |   4 +-
 drivers/infiniband/hw/qib/qib_fs.c   | 105 ++++++---------------------
 drivers/infiniband/hw/qib/qib_init.c |  32 +++-----
 3 files changed, 36 insertions(+), 105 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib.h b/drivers/infiniband/hw/qib/qib.h
index 26c615772be390..f73c321d0bff88 100644
--- a/drivers/infiniband/hw/qib/qib.h
+++ b/drivers/infiniband/hw/qib/qib.h
@@ -1370,10 +1370,10 @@ void qib_device_remove(struct qib_devdata *);
 extern int qib_qsfp_dump(struct qib_pportdata *ppd, char *buf, int len);
 
 int __init qib_init_qibfs(void);
-int __exit qib_exit_qibfs(void);
+void __exit qib_exit_qibfs(void);
 
 int qibfs_add(struct qib_devdata *);
-int qibfs_remove(struct qib_devdata *);
+void qibfs_remove(struct qib_devdata *);
 
 int qib_pcie_init(struct pci_dev *, const struct pci_device_id *);
 int qib_pcie_ddinit(struct qib_devdata *, struct pci_dev *,
diff --git a/drivers/infiniband/hw/qib/qib_fs.c b/drivers/infiniband/hw/qib/qib_fs.c
index ed7d4b02f45a63..c52ca34b32e67d 100644
--- a/drivers/infiniband/hw/qib/qib_fs.c
+++ b/drivers/infiniband/hw/qib/qib_fs.c
@@ -43,7 +43,9 @@
 
 #define QIBFS_MAGIC 0x726a77
 
-static struct super_block *qib_super;
+static struct file_system_type qibfs_fs_type;
+static struct vfsmount *qib_mount;
+static int qib_mnt_count;
 
 #define private2dd(file) (file_inode(file)->i_private)
 
@@ -355,15 +357,19 @@ static const struct file_operations flash_ops = {
 	.llseek = default_llseek,
 };
 
-static int add_cntr_files(struct super_block *sb, struct qib_devdata *dd)
+int qibfs_add(struct qib_devdata *dd)
 {
 	struct dentry *dir, *tmp;
 	char unit[10];
 	int ret, i;
+	
+	ret = simple_pin_fs(&qibfs_fs_type, &qib_mount, &qib_mnt_count);
+	if (ret)
+		return ret;
 
 	/* create the per-unit directory */
 	snprintf(unit, sizeof(unit), "%u", dd->unit);
-	ret = create_file(unit, S_IFDIR|S_IRUGO|S_IXUGO, sb->s_root, &dir,
+	ret = create_file(unit, S_IFDIR|S_IRUGO|S_IXUGO, qib_mount->mnt_root, &dir,
 			  &simple_dir_operations, dd);
 	if (ret) {
 		pr_err("create_file(%s) failed: %d\n", unit, ret);
@@ -422,65 +428,35 @@ static int add_cntr_files(struct super_block *sb, struct qib_devdata *dd)
 		pr_err("create_file(%s/flash) failed: %d\n",
 			unit, ret);
 bail:
+	simple_release_fs(&qib_mount, &qib_mnt_count);
 	return ret;
 }
 
-static int remove_device_files(struct super_block *sb,
-			       struct qib_devdata *dd)
+void qibfs_remove(struct qib_devdata *dd)
 {
 	struct dentry *dir;
 	char unit[10];
 
 	snprintf(unit, sizeof(unit), "%u", dd->unit);
-	dir = lookup_one_len_unlocked(unit, sb->s_root, strlen(unit));
-
-	if (IS_ERR(dir)) {
-		pr_err("Lookup of %s failed\n", unit);
-		return PTR_ERR(dir);
-	}
-	simple_recursive_removal(dir, NULL);
-	return 0;
+	dir = lookup_one_len_unlocked(unit, qib_mount->mnt_root, strlen(unit));
+	if (!IS_ERR(dir))
+		simple_recursive_removal(dir, NULL);
+	simple_release_fs(&qib_mount, &qib_mnt_count);
 }
 
-/*
- * This fills everything in when the fs is mounted, to handle umount/mount
- * after device init.  The direct add_cntr_files() call handles adding
- * them from the init code, when the fs is already mounted.
- */
 static int qibfs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
-	struct qib_devdata *dd;
-	unsigned long index;
-	int ret;
-
 	static const struct tree_descr files[] = {
 		[2] = {"driver_stats", &driver_ops[0], S_IRUGO},
 		[3] = {"driver_stats_names", &driver_ops[1], S_IRUGO},
 		{""},
 	};
-
-	ret = simple_fill_super(sb, QIBFS_MAGIC, files);
-	if (ret) {
-		pr_err("simple_fill_super failed: %d\n", ret);
-		goto bail;
-	}
-
-	xa_for_each(&qib_dev_table, index, dd) {
-		ret = add_cntr_files(sb, dd);
-		if (ret)
-			goto bail;
-	}
-
-bail:
-	return ret;
+	return simple_fill_super(sb, QIBFS_MAGIC, files);
 }
 
 static int qibfs_get_tree(struct fs_context *fc)
 {
-	int ret = get_tree_single(fc, qibfs_fill_super);
-	if (ret == 0)
-		qib_super = fc->root->d_sb;
-	return ret;
+	return get_tree_single(fc, qibfs_fill_super);
 }
 
 static const struct fs_context_operations qibfs_context_ops = {
@@ -493,46 +469,11 @@ static int qibfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void qibfs_kill_super(struct super_block *s)
-{
-	kill_litter_super(s);
-	qib_super = NULL;
-}
-
-int qibfs_add(struct qib_devdata *dd)
-{
-	int ret;
-
-	/*
-	 * On first unit initialized, qib_super will not yet exist
-	 * because nobody has yet tried to mount the filesystem, so
-	 * we can't consider that to be an error; if an error occurs
-	 * during the mount, that will get a complaint, so this is OK.
-	 * add_cntr_files() for all units is done at mount from
-	 * qibfs_fill_super(), so one way or another, everything works.
-	 */
-	if (qib_super == NULL)
-		ret = 0;
-	else
-		ret = add_cntr_files(qib_super, dd);
-	return ret;
-}
-
-int qibfs_remove(struct qib_devdata *dd)
-{
-	int ret = 0;
-
-	if (qib_super)
-		ret = remove_device_files(qib_super, dd);
-
-	return ret;
-}
-
 static struct file_system_type qibfs_fs_type = {
-	.owner =        THIS_MODULE,
-	.name =         "ipathfs",
-	.init_fs_context = qibfs_init_fs_context,
-	.kill_sb =      qibfs_kill_super,
+	.owner			= THIS_MODULE,
+	.name			= "ipathfs",
+	.init_fs_context	= qibfs_init_fs_context,
+	.kill_sb		= kill_litter_super,
 };
 MODULE_ALIAS_FS("ipathfs");
 
@@ -541,7 +482,7 @@ int __init qib_init_qibfs(void)
 	return register_filesystem(&qibfs_fs_type);
 }
 
-int __exit qib_exit_qibfs(void)
+void __exit qib_exit_qibfs(void)
 {
-	return unregister_filesystem(&qibfs_fs_type);
+	unregister_filesystem(&qibfs_fs_type);
 }
diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c
index 33667becd52b04..46306573a37a7d 100644
--- a/drivers/infiniband/hw/qib/qib_init.c
+++ b/drivers/infiniband/hw/qib/qib_init.c
@@ -1217,9 +1217,13 @@ static int __init qib_ib_init(void)
 {
 	int ret;
 
+	ret = qib_init_qibfs();
+	if (ret)
+		return ret;
+
 	ret = qib_dev_init();
 	if (ret)
-		goto bail;
+		goto bail_fs;
 
 	/*
 	 * These must be called before the driver is registered with
@@ -1237,10 +1241,7 @@ static int __init qib_ib_init(void)
 		goto bail_dev;
 	}
 
-	/* not fatal if it doesn't work */
-	if (qib_init_qibfs())
-		pr_err("Unable to register ipathfs\n");
-	goto bail; /* all OK */
+	return ret;
 
 bail_dev:
 #ifdef CONFIG_INFINIBAND_QIB_DCA
@@ -1250,7 +1251,8 @@ static int __init qib_ib_init(void)
 	qib_dbg_exit();
 #endif
 	qib_dev_cleanup();
-bail:
+bail_fs:
+	qib_exit_qibfs();
 	return ret;
 }
 
@@ -1261,14 +1263,6 @@ module_init(qib_ib_init);
  */
 static void __exit qib_ib_cleanup(void)
 {
-	int ret;
-
-	ret = qib_exit_qibfs();
-	if (ret)
-		pr_err(
-			"Unable to cleanup counter filesystem: error %d\n",
-			-ret);
-
 #ifdef CONFIG_INFINIBAND_QIB_DCA
 	dca_unregister_notify(&dca_notifier);
 #endif
@@ -1282,6 +1276,7 @@ static void __exit qib_ib_cleanup(void)
 
 	WARN_ON(!xa_empty(&qib_dev_table));
 	qib_dev_cleanup();
+	qib_exit_qibfs();
 }
 
 module_exit(qib_ib_cleanup);
@@ -1469,7 +1464,7 @@ static int qib_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		if (qib_mini_init)
 			goto bail;
 		if (!j) {
-			(void) qibfs_remove(dd);
+			qibfs_remove(dd);
 			qib_device_remove(dd);
 		}
 		if (!ret)
@@ -1496,7 +1491,6 @@ static int qib_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 static void qib_remove_one(struct pci_dev *pdev)
 {
 	struct qib_devdata *dd = pci_get_drvdata(pdev);
-	int ret;
 
 	/* unregister from IB core */
 	qib_unregister_ib_device(dd);
@@ -1513,11 +1507,7 @@ static void qib_remove_one(struct pci_dev *pdev)
 	/* wait until all of our (qsfp) queue_work() calls complete */
 	flush_workqueue(ib_wq);
 
-	ret = qibfs_remove(dd);
-	if (ret)
-		qib_dev_err(dd, "Failed counters filesystem cleanup: %d\n",
-			    -ret);
-
+	qibfs_remove(dd);
 	qib_device_remove(dd);
 
 	qib_postinit_cleanup(dd);
-- 
2.39.2

Re: [PATCH 06/19] qibfs: use simple_release_fs

[Leon Romanovsky]

On Wed, Sep 13, 2023 at 08:10:00AM -0300, Christoph Hellwig wrote:
> qibfs currently has convoluted code to allow registering HCAs while qibfs
> is not mounted and vice versa.  Switch to using simple_release_fs every
> time an entry is added to pin the fs instance and remove all the boiler
> plate code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/infiniband/hw/qib/qib.h      |   4 +-
>  drivers/infiniband/hw/qib/qib_fs.c   | 105 ++++++---------------------
>  drivers/infiniband/hw/qib/qib_init.c |  32 +++-----
>  3 files changed, 36 insertions(+), 105 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leon@kernel.org>

[PATCH 07/19] hypfs: use d_genocide to kill fs entries

[Christoph Hellwig]

hypfs is entirely synthetic and doesn't care about i_nlink when dropping
entries from the cache.  Switch to d_genocide instead of a home grown
file remove loop for unmount and write (yes, really!).

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/s390/hypfs/inode.c | 37 ++-----------------------------------
 1 file changed, 2 insertions(+), 35 deletions(-)

diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c
index dbe8a7dcafa922..3261fb9cade648 100644
--- a/arch/s390/hypfs/inode.c
+++ b/arch/s390/hypfs/inode.c
@@ -64,33 +64,6 @@ static void hypfs_add_dentry(struct dentry *dentry)
 	hypfs_last_dentry = dentry;
 }
 
-static void hypfs_remove(struct dentry *dentry)
-{
-	struct dentry *parent;
-
-	parent = dentry->d_parent;
-	inode_lock(d_inode(parent));
-	if (simple_positive(dentry)) {
-		if (d_is_dir(dentry))
-			simple_rmdir(d_inode(parent), dentry);
-		else
-			simple_unlink(d_inode(parent), dentry);
-	}
-	d_drop(dentry);
-	dput(dentry);
-	inode_unlock(d_inode(parent));
-}
-
-static void hypfs_delete_tree(struct dentry *root)
-{
-	while (hypfs_last_dentry) {
-		struct dentry *next_dentry;
-		next_dentry = hypfs_last_dentry->d_fsdata;
-		hypfs_remove(hypfs_last_dentry);
-		hypfs_last_dentry = next_dentry;
-	}
-}
-
 static struct inode *hypfs_make_inode(struct super_block *sb, umode_t mode)
 {
 	struct inode *ret = new_inode(sb);
@@ -183,14 +156,14 @@ static ssize_t hypfs_write_iter(struct kiocb *iocb, struct iov_iter *from)
 		rc = -EBUSY;
 		goto out;
 	}
-	hypfs_delete_tree(sb->s_root);
+	d_genocide(sb->s_root);
 	if (MACHINE_IS_VM)
 		rc = hypfs_vm_create_files(sb->s_root);
 	else
 		rc = hypfs_diag_create_files(sb->s_root);
 	if (rc) {
 		pr_err("Updating the hypfs tree failed\n");
-		hypfs_delete_tree(sb->s_root);
+		d_genocide(sb->s_root);
 		goto out;
 	}
 	hypfs_update_update(sb);
@@ -323,12 +296,6 @@ static int hypfs_init_fs_context(struct fs_context *fc)
 
 static void hypfs_kill_super(struct super_block *sb)
 {
-	struct hypfs_sb_info *sb_info = sb->s_fs_info;
-
-	if (sb->s_root)
-		hypfs_delete_tree(sb->s_root);
-	if (sb_info && sb_info->update_file)
-		hypfs_remove(sb_info->update_file);
 	kill_litter_super(sb);
 	kfree(sb->s_fs_info);
 }
-- 
2.39.2

[PATCH 08/19] pstore: shrink the pstore_sb_lock critical section in pstore_kill_sb

[Christoph Hellwig]

->kill_sb can't race with creating ->fill_super because pstore is a
_single file system that only ever has a single sb instance, and we wait
for the previous one to go away before creating a new one.  Reduce
the critical section so that is is not held over generic_shutdown_super.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/pstore/inode.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 585360706b335f..fd1d24b47160d0 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -467,10 +467,9 @@ static struct dentry *pstore_mount(struct file_system_type *fs_type,
 
 static void pstore_kill_sb(struct super_block *sb)
 {
-	mutex_lock(&pstore_sb_lock);
-	WARN_ON(pstore_sb && pstore_sb != sb);
-
 	kill_litter_super(sb);
+
+	mutex_lock(&pstore_sb_lock);
 	pstore_sb = NULL;
 
 	mutex_lock(&records_list_lock);
-- 
2.39.2

Re: [PATCH 08/19] pstore: shrink the pstore_sb_lock critical section in pstore_kill_sb

[Kees Cook]

On Wed, Sep 13, 2023 at 08:10:02AM -0300, Christoph Hellwig wrote:
> ->kill_sb can't race with creating ->fill_super because pstore is a
> _single file system that only ever has a single sb instance, and we wait
> for the previous one to go away before creating a new one.  Reduce
> the critical section so that is is not held over generic_shutdown_super.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Thanks for the refactoring!

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

[PATCH 09/19] zonefs: remove duplicate cleanup in zonefs_fill_super

[Christoph Hellwig]

When ->fill_super fails, ->kill_sb is called which already cleans up
the inodes and zgroups.

Drop the extra cleanup code in zonefs_fill_super.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/zonefs/super.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
index 9d1a9808fbbba6..35b2554ce2ac2e 100644
--- a/fs/zonefs/super.c
+++ b/fs/zonefs/super.c
@@ -1309,13 +1309,12 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
 	/* Initialize the zone groups */
 	ret = zonefs_init_zgroups(sb);
 	if (ret)
-		goto cleanup;
+		return ret;
 
 	/* Create the root directory inode */
-	ret = -ENOMEM;
 	inode = new_inode(sb);
 	if (!inode)
-		goto cleanup;
+		return -ENOMEM;
 
 	inode->i_ino = bdev_nr_zones(sb->s_bdev);
 	inode->i_mode = S_IFDIR | 0555;
@@ -1333,7 +1332,7 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
 
 	sb->s_root = d_make_root(inode);
 	if (!sb->s_root)
-		goto cleanup;
+		return -ENOMEM;
 
 	/*
 	 * Take a reference on the zone groups directory inodes
@@ -1341,19 +1340,9 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
 	 */
 	ret = zonefs_get_zgroup_inodes(sb);
 	if (ret)
-		goto cleanup;
-
-	ret = zonefs_sysfs_register(sb);
-	if (ret)
-		goto cleanup;
-
-	return 0;
-
-cleanup:
-	zonefs_release_zgroup_inodes(sb);
-	zonefs_free_zgroups(sb);
+		return ret;
 
-	return ret;
+	return zonefs_sysfs_register(sb);
 }
 
 static struct dentry *zonefs_mount(struct file_system_type *fs_type,
-- 
2.39.2

Re: [PATCH 09/19] zonefs: remove duplicate cleanup in zonefs_fill_super

[Damien Le Moal]

On 9/13/23 20:10, Christoph Hellwig wrote:
> When ->fill_super fails, ->kill_sb is called which already cleans up
> the inodes and zgroups.
> 
> Drop the extra cleanup code in zonefs_fill_super.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me.

Acked-by: Damien Le Moal <dlemoal@kernel.org>

> ---
>  fs/zonefs/super.c | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
> index 9d1a9808fbbba6..35b2554ce2ac2e 100644
> --- a/fs/zonefs/super.c
> +++ b/fs/zonefs/super.c
> @@ -1309,13 +1309,12 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
>  	/* Initialize the zone groups */
>  	ret = zonefs_init_zgroups(sb);
>  	if (ret)
> -		goto cleanup;
> +		return ret;
>  
>  	/* Create the root directory inode */
> -	ret = -ENOMEM;
>  	inode = new_inode(sb);
>  	if (!inode)
> -		goto cleanup;
> +		return -ENOMEM;
>  
>  	inode->i_ino = bdev_nr_zones(sb->s_bdev);
>  	inode->i_mode = S_IFDIR | 0555;
> @@ -1333,7 +1332,7 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	sb->s_root = d_make_root(inode);
>  	if (!sb->s_root)
> -		goto cleanup;
> +		return -ENOMEM;
>  
>  	/*
>  	 * Take a reference on the zone groups directory inodes
> @@ -1341,19 +1340,9 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
>  	 */
>  	ret = zonefs_get_zgroup_inodes(sb);
>  	if (ret)
> -		goto cleanup;
> -
> -	ret = zonefs_sysfs_register(sb);
> -	if (ret)
> -		goto cleanup;
> -
> -	return 0;
> -
> -cleanup:
> -	zonefs_release_zgroup_inodes(sb);
> -	zonefs_free_zgroups(sb);
> +		return ret;
>  
> -	return ret;
> +	return zonefs_sysfs_register(sb);
>  }
>  
>  static struct dentry *zonefs_mount(struct file_system_type *fs_type,

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 09/19] zonefs: remove duplicate cleanup in zonefs_fill_super

[Al Viro]

On Wed, Sep 13, 2023 at 08:10:03AM -0300, Christoph Hellwig wrote:
> When ->fill_super fails, ->kill_sb is called which already cleans up
> the inodes and zgroups.

Ugh...  The use of "->" strongly suggests that you are talking about
a method; 'fill_super' here actually refers to callback passed to
mount_bdev().  Have a pity for those who'll be trying to parse it
- that might be yourself a couple of years down the road...

Something like

"If zonefs_fill_super() returns an error, its caller (mount_bdev()) will
make sure to call zonefs_kill_super(), which already cleans up
the inodes and zgroups.", perhaps?

> 
> Drop the extra cleanup code in zonefs_fill_super.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/zonefs/super.c | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
> index 9d1a9808fbbba6..35b2554ce2ac2e 100644
> --- a/fs/zonefs/super.c
> +++ b/fs/zonefs/super.c
> @@ -1309,13 +1309,12 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
>  	/* Initialize the zone groups */
>  	ret = zonefs_init_zgroups(sb);
>  	if (ret)
> -		goto cleanup;
> +		return ret;
>  
>  	/* Create the root directory inode */
> -	ret = -ENOMEM;
>  	inode = new_inode(sb);
>  	if (!inode)
> -		goto cleanup;
> +		return -ENOMEM;
>  
>  	inode->i_ino = bdev_nr_zones(sb->s_bdev);
>  	inode->i_mode = S_IFDIR | 0555;
> @@ -1333,7 +1332,7 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	sb->s_root = d_make_root(inode);
>  	if (!sb->s_root)
> -		goto cleanup;
> +		return -ENOMEM;
>  
>  	/*
>  	 * Take a reference on the zone groups directory inodes
> @@ -1341,19 +1340,9 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
>  	 */
>  	ret = zonefs_get_zgroup_inodes(sb);
>  	if (ret)
> -		goto cleanup;
> -
> -	ret = zonefs_sysfs_register(sb);
> -	if (ret)
> -		goto cleanup;
> -
> -	return 0;
> -
> -cleanup:
> -	zonefs_release_zgroup_inodes(sb);
> -	zonefs_free_zgroups(sb);
> +		return ret;
>  
> -	return ret;
> +	return zonefs_sysfs_register(sb);
>  }
>  
>  static struct dentry *zonefs_mount(struct file_system_type *fs_type,
> -- 
> 2.39.2
> 

[PATCH 10/19] USB: gadget/legacy: remove sb_mutex

[Christoph Hellwig]

Creating new a new super_block vs freeing the old one for single instance
file systems is serialized by the wait for SB_DEAD.

Remove the superfluous sb_mutex.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/usb/gadget/legacy/inode.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index ce9e31f3d26bcc..a203266bc0dc82 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -229,7 +229,6 @@ static void put_ep (struct ep_data *data)
  */
 
 static const char *CHIP;
-static DEFINE_MUTEX(sb_mutex);		/* Serialize superblock operations */
 
 /*----------------------------------------------------------------------*/
 
@@ -2012,8 +2011,6 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
 	struct dev_data	*dev;
 	int		rc;
 
-	mutex_lock(&sb_mutex);
-
 	if (the_device) {
 		rc = -ESRCH;
 		goto Done;
@@ -2069,7 +2066,6 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
 	rc = -ENOMEM;
 
  Done:
-	mutex_unlock(&sb_mutex);
 	return rc;
 }
 
@@ -2092,7 +2088,6 @@ static int gadgetfs_init_fs_context(struct fs_context *fc)
 static void
 gadgetfs_kill_sb (struct super_block *sb)
 {
-	mutex_lock(&sb_mutex);
 	kill_litter_super (sb);
 	if (the_device) {
 		put_dev (the_device);
@@ -2100,7 +2095,6 @@ gadgetfs_kill_sb (struct super_block *sb)
 	}
 	kfree(CHIP);
 	CHIP = NULL;
-	mutex_unlock(&sb_mutex);
 }
 
 /*----------------------------------------------------------------------*/
-- 
2.39.2

Re: [PATCH 10/19] USB: gadget/legacy: remove sb_mutex

[Alan Stern]

On Wed, Sep 13, 2023 at 08:10:04AM -0300, Christoph Hellwig wrote:
> Creating new a new super_block vs freeing the old one for single instance
> file systems is serialized by the wait for SB_DEAD.
> 
> Remove the superfluous sb_mutex.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

You might mention that this is essentially a reversion of commit 
d18dcfe9860e ("USB: gadgetfs: Fix race between mounting and 
unmounting").

Alan Stern

>  drivers/usb/gadget/legacy/inode.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
> index ce9e31f3d26bcc..a203266bc0dc82 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
> @@ -229,7 +229,6 @@ static void put_ep (struct ep_data *data)
>   */
>  
>  static const char *CHIP;
> -static DEFINE_MUTEX(sb_mutex);		/* Serialize superblock operations */
>  
>  /*----------------------------------------------------------------------*/
>  
> @@ -2012,8 +2011,6 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
>  	struct dev_data	*dev;
>  	int		rc;
>  
> -	mutex_lock(&sb_mutex);
> -
>  	if (the_device) {
>  		rc = -ESRCH;
>  		goto Done;
> @@ -2069,7 +2066,6 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
>  	rc = -ENOMEM;
>  
>   Done:
> -	mutex_unlock(&sb_mutex);
>  	return rc;
>  }
>  
> @@ -2092,7 +2088,6 @@ static int gadgetfs_init_fs_context(struct fs_context *fc)
>  static void
>  gadgetfs_kill_sb (struct super_block *sb)
>  {
> -	mutex_lock(&sb_mutex);
>  	kill_litter_super (sb);
>  	if (the_device) {
>  		put_dev (the_device);
> @@ -2100,7 +2095,6 @@ gadgetfs_kill_sb (struct super_block *sb)
>  	}
>  	kfree(CHIP);
>  	CHIP = NULL;
> -	mutex_unlock(&sb_mutex);
>  }
>  
>  /*----------------------------------------------------------------------*/
> -- 
> 2.39.2
> 

Re: [PATCH 10/19] USB: gadget/legacy: remove sb_mutex

[Christoph Hellwig]

On Wed, Sep 13, 2023 at 12:10:56PM -0400, Alan Stern wrote:
> On Wed, Sep 13, 2023 at 08:10:04AM -0300, Christoph Hellwig wrote:
> > Creating new a new super_block vs freeing the old one for single instance
> > file systems is serialized by the wait for SB_DEAD.
> > 
> > Remove the superfluous sb_mutex.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> 
> You might mention that this is essentially a reversion of commit 
> d18dcfe9860e ("USB: gadgetfs: Fix race between mounting and 
> unmounting").

The difference we have now is that sb creation waits for the
old one to go way, but I'll add a notice on all that.

Re: [PATCH 10/19] USB: gadget/legacy: remove sb_mutex

[Sergey Shtylyov]

On 9/13/23 2:10 PM, Christoph Hellwig wrote:

> Creating new a new super_block vs freeing the old one for single instance
           ^^^^^^^^^
   I can't parse that. :-)

> file systems is serialized by the wait for SB_DEAD.
> 
> Remove the superfluous sb_mutex.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
[...]

MBR, Sergey

[PATCH 11/19] fs: add new shutdown_sb and free_sb methods

[Christoph Hellwig]

Currently super_blocks are shut down using the ->kill_sb method, which
must call generic_shutdown_super, but allows the file system to
add extra work before or after the call to generic_shutdown_super.

File systems tend to get rather confused by this, so add an alternative
shutdown sequence where generic_shutdown_super is called by the core
code, and there are extra ->shutdown_sb and ->free_sb hooks before and
after it.  To remove the amount of boilerplate code ->shutdown_sb is only
called if the super has finished initialization and ->d_root is set.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/filesystems/locking.rst |  4 ++++
 Documentation/filesystems/vfs.rst     | 12 ++++++++++++
 fs/super.c                            |  9 +++++++--
 include/linux/fs.h                    |  2 ++
 4 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
index 7be2900806c853..c33e2f03ed1f69 100644
--- a/Documentation/filesystems/locking.rst
+++ b/Documentation/filesystems/locking.rst
@@ -220,7 +220,9 @@ prototypes::
 
 	struct dentry *(*mount) (struct file_system_type *, int,
 		       const char *, void *);
+	void (*shutdown_sb) (struct super_block *);
 	void (*kill_sb) (struct super_block *);
+	void (*free_sb) (struct super_block *);
 
 locking rules:
 
@@ -228,7 +230,9 @@ locking rules:
 ops		may block
 =======		=========
 mount		yes
+shutdown_sb	yes
 kill_sb		yes
+free_sb		yes
 =======		=========
 
 ->mount() returns ERR_PTR or the root dentry; its superblock should be locked
diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst
index 99acc2e9867391..1a7c6926c31f34 100644
--- a/Documentation/filesystems/vfs.rst
+++ b/Documentation/filesystems/vfs.rst
@@ -119,7 +119,9 @@ members are defined:
 		const struct fs_parameter_spec *parameters;
 		struct dentry *(*mount) (struct file_system_type *, int,
 			const char *, void *);
+		void (*shutdown_sb) (struct super_block *);
 		void (*kill_sb) (struct super_block *);
+		void (*free_sb) (struct super_block *);
 		struct module *owner;
 		struct file_system_type * next;
 		struct hlist_head fs_supers;
@@ -155,10 +157,20 @@ members are defined:
 	the method to call when a new instance of this filesystem should
 	be mounted
 
+``shutdown_sb``
+	Cleanup after a super_block has reached a zero active count, and before
+	the VFS level cleanup happens.  Typical picks all fs-specific objects
+	(if any) that need destruction out of superblock and releases them.
+	Note: dentries and inodes are normally taken care of and do not need
+	specific handling unless they are pinned by kernel users.
+
 ``kill_sb``
 	the method to call when an instance of this filesystem should be
 	shut down
 
+``free_sb``
+	Free file system specific resources like sb->s_fs_info that are
+	still needed while inodes are freed during umount.
 
 ``owner``
 	for internal VFS use: you should initialize this to THIS_MODULE
diff --git a/fs/super.c b/fs/super.c
index 5c685b4944c2d6..8e173eccc8c113 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -480,10 +480,15 @@ void deactivate_locked_super(struct super_block *s)
 
 	unregister_shrinker(&s->s_shrink);
 
-	if (fs->kill_sb)
+	if (fs->kill_sb) {
 		fs->kill_sb(s);
-	else
+	} else {
+		if (fs->shutdown_sb)
+			fs->shutdown_sb(s);
 		generic_shutdown_super(s);
+		if (fs->free_sb)
+			fs->free_sb(s);
+	}
 
 	kill_super_notify(s);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 31b6b235b36efa..12fff7df3cc46b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2341,6 +2341,8 @@ struct file_system_type {
 	struct dentry *(*mount) (struct file_system_type *, int,
 		       const char *, void *);
 	void (*kill_sb) (struct super_block *);
+	void (*shutdown_sb)(struct super_block *sb);
+	void (*free_sb)(struct super_block *sb);
 	struct module *owner;
 	struct file_system_type * next;
 	struct hlist_head fs_supers;
-- 
2.39.2

Re: [PATCH 11/19] fs: add new shutdown_sb and free_sb methods

[Al Viro]

On Wed, Sep 13, 2023 at 08:10:05AM -0300, Christoph Hellwig wrote:
> Currently super_blocks are shut down using the ->kill_sb method, which
> must call generic_shutdown_super, but allows the file system to
> add extra work before or after the call to generic_shutdown_super.
> 
> File systems tend to get rather confused by this, so add an alternative
> shutdown sequence where generic_shutdown_super is called by the core
> code, and there are extra ->shutdown_sb and ->free_sb hooks before and
> after it.  To remove the amount of boilerplate code ->shutdown_sb is only
> called if the super has finished initialization and ->d_root is set.

The last sentence doesn't match the patchset.  That aside, there
is an issue with method names.

->shutdown_sb() is... odd.  ->begin_shutdown_sb(), perhaps?  For the
majority of filesystems it's NULL, after all...

Worse, ->free_sb() is seriously misguiding - the name implies that
we are, well, freeing a superblock passed to it.  Which is not what is
happening here - superblock itself is freed only when all passive
references go away.  It's asking for trouble down the road.

We already have more than enough confusion in the area.  Note, BTW,
that there's a delicate issue around RCU accesses and freeing stuff -
->d_compare() can bloody well be called when superblock is getting
shut down.  For anything that might be needed by it (or by other
RCU'd methods) we must arrange for RCU-delayed destruction.
E.g. in case of fatfs we have sbi freeing done via call_rcu() (from
fat_put_super(), called by generic_shutdown_super()).

<checks the current tree>

Oh, bugger...  AFAICS, exfat has a problem - exfat_free_sbi() is called
directly from exfat_kill_sb(), without any concern for this:
static int exfat_utf8_d_cmp(const struct dentry *dentry, unsigned int len,
                const char *str, const struct qstr *name)
{
        struct super_block *sb = dentry->d_sb;
        unsigned int alen = exfat_striptail_len(name->len, name->name,
                                EXFAT_SB(sb)->options.keep_last_dots);

That kfree() needs to be RCU-delayed...  While we are at it, there's
this:
static int exfat_d_hash(const struct dentry *dentry, struct qstr *qstr)
{
        struct super_block *sb = dentry->d_sb;
        struct nls_table *t = EXFAT_SB(sb)->nls_io;
and we need this
        unload_nls(sbi->nls_io);
in exfat_put_super() RCU-delayed as well.  And I suspect that
        exfat_free_upcase_table(sbi);
right after it needs the same treatment.

AFFS: similar problem, wants ->s_fs_info freeing RCU-delayed.

hfsplus: similar, including non-delayed unlock_nls() calls.

ntfs3:
        /*
         * Try slow way with current upcase table
         */
        sbi = dentry->d_sb->s_fs_info;
        uni1 = __getname();
        if (!uni1)
                return -ENOMEM;
__getname().  "Give me a page and you might block, while you are
at it".  Done from ->d_compare().  Called under dentry->d_lock
and rcu_read_lock().  OK, any further investigation of that
one is... probably not worth bothering with at that point.

Other in-tree instances appear to be correct.  I'll push fixes for
those (well, ntfs3 aside) out tomorrow.

[PATCH 12/19] fs: convert kill_litter_super to litter_shutdown_sb

[Christoph Hellwig]

Replace kill_litter_super with litter_shutdown_sb, which is wired up to
the ->shutdown_sb method.  For file systems that wrapped
kill_litter_super, ->kill_sb is replaced with ->shutdown and ->free_sb
methods as needed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/platforms/cell/spufs/inode.c | 10 +++++-----
 arch/s390/hypfs/inode.c                   |  6 +++---
 drivers/android/binderfs.c                | 12 +++---------
 drivers/base/devtmpfs.c                   |  8 ++++----
 drivers/infiniband/hw/qib/qib_fs.c        |  2 +-
 drivers/misc/ibmasm/ibmasmfs.c            |  8 ++++----
 drivers/usb/gadget/function/f_fs.c        |  6 +++---
 drivers/usb/gadget/legacy/inode.c         | 12 ++++++------
 drivers/xen/xenfs/super.c                 |  8 ++++----
 fs/binfmt_misc.c                          |  8 ++++----
 fs/configfs/mount.c                       |  8 ++++----
 fs/debugfs/inode.c                        |  8 ++++----
 fs/devpts/inode.c                         |  6 +++---
 fs/efivarfs/super.c                       | 13 ++++++-------
 fs/fuse/control.c                         | 12 ++++++------
 fs/hugetlbfs/inode.c                      |  2 +-
 fs/nfsd/nfsctl.c                          | 22 ++++++++++++----------
 fs/ocfs2/dlmfs/dlmfs.c                    |  2 +-
 fs/pstore/inode.c                         |  7 +++----
 fs/ramfs/inode.c                          |  6 +++---
 fs/super.c                                | 14 +++++++++++---
 fs/tracefs/inode.c                        |  2 +-
 include/linux/fs.h                        |  2 +-
 include/linux/ramfs.h                     |  2 +-
 init/do_mounts.c                          |  6 +++---
 ipc/mqueue.c                              |  2 +-
 kernel/bpf/inode.c                        |  2 +-
 mm/shmem.c                                |  5 +++--
 net/sunrpc/rpc_pipe.c                     | 19 ++++++++++++-------
 security/inode.c                          |  8 ++++----
 security/selinux/selinuxfs.c              | 15 +++++----------
 security/smack/smackfs.c                  |  6 +++---
 32 files changed, 126 insertions(+), 123 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index 38c5be34c8951f..2610a0731ea242 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -763,11 +763,11 @@ static int spufs_init_fs_context(struct fs_context *fc)
 }
 
 static struct file_system_type spufs_type = {
-	.owner = THIS_MODULE,
-	.name = "spufs",
-	.init_fs_context = spufs_init_fs_context,
-	.parameters	= spufs_fs_parameters,
-	.kill_sb = kill_litter_super,
+	.owner			= THIS_MODULE,
+	.name			= "spufs",
+	.init_fs_context	= spufs_init_fs_context,
+	.parameters		= spufs_fs_parameters,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 MODULE_ALIAS_FS("spufs");
 
diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c
index 3261fb9cade648..f18e3b844c5d9b 100644
--- a/arch/s390/hypfs/inode.c
+++ b/arch/s390/hypfs/inode.c
@@ -294,9 +294,8 @@ static int hypfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void hypfs_kill_super(struct super_block *sb)
+static void hypfs_free_sb(struct super_block *sb)
 {
-	kill_litter_super(sb);
 	kfree(sb->s_fs_info);
 }
 
@@ -417,7 +416,8 @@ static struct file_system_type hypfs_type = {
 	.name		= "s390_hypfs",
 	.init_fs_context = hypfs_init_fs_context,
 	.parameters	= hypfs_fs_parameters,
-	.kill_sb	= hypfs_kill_super
+	.shutdown_sb	= litter_shutdown_sb,
+	.free_sb	= hypfs_free_sb,
 };
 
 static const struct super_operations hypfs_s_ops = {
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 81effec17b3d63..f48196391239c0 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -771,19 +771,12 @@ static int binderfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void binderfs_kill_super(struct super_block *sb)
+static void binderfs_free_sb(struct super_block *sb)
 {
 	struct binderfs_info *info = sb->s_fs_info;
 
-	/*
-	 * During inode eviction struct binderfs_info is needed.
-	 * So first wipe the super_block then free struct binderfs_info.
-	 */
-	kill_litter_super(sb);
-
 	if (info && info->ipc_ns)
 		put_ipc_ns(info->ipc_ns);
-
 	kfree(info);
 }
 
@@ -791,7 +784,8 @@ static struct file_system_type binder_fs_type = {
 	.name			= "binder",
 	.init_fs_context	= binderfs_init_fs_context,
 	.parameters		= binderfs_fs_parameters,
-	.kill_sb		= binderfs_kill_super,
+	.shutdown_sb		= litter_shutdown_sb,
+	.free_sb		= binderfs_free_sb,
 	.fs_flags		= FS_USERNS_MOUNT,
 };
 
diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index b848764ef0187f..f5b43c7877815b 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -80,13 +80,13 @@ static struct dentry *public_dev_mount(struct file_system_type *fs_type, int fla
 }
 
 static struct file_system_type internal_fs_type = {
-	.name = "devtmpfs",
+	.name			= "devtmpfs",
 #ifdef CONFIG_TMPFS
-	.init_fs_context = shmem_init_fs_context,
+	.init_fs_context	= shmem_init_fs_context,
 #else
-	.init_fs_context = ramfs_init_fs_context,
+	.init_fs_context	= ramfs_init_fs_context,
 #endif
-	.kill_sb = kill_litter_super,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 
 static struct file_system_type dev_fs_type = {
diff --git a/drivers/infiniband/hw/qib/qib_fs.c b/drivers/infiniband/hw/qib/qib_fs.c
index c52ca34b32e67d..ea0aeade92bbd0 100644
--- a/drivers/infiniband/hw/qib/qib_fs.c
+++ b/drivers/infiniband/hw/qib/qib_fs.c
@@ -473,7 +473,7 @@ static struct file_system_type qibfs_fs_type = {
 	.owner			= THIS_MODULE,
 	.name			= "ipathfs",
 	.init_fs_context	= qibfs_init_fs_context,
-	.kill_sb		= kill_litter_super,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 MODULE_ALIAS_FS("ipathfs");
 
diff --git a/drivers/misc/ibmasm/ibmasmfs.c b/drivers/misc/ibmasm/ibmasmfs.c
index 5867af9f592cdb..05d03f9b600366 100644
--- a/drivers/misc/ibmasm/ibmasmfs.c
+++ b/drivers/misc/ibmasm/ibmasmfs.c
@@ -100,10 +100,10 @@ static const struct super_operations ibmasmfs_s_ops = {
 static const struct file_operations *ibmasmfs_dir_ops = &simple_dir_operations;
 
 static struct file_system_type ibmasmfs_type = {
-	.owner          = THIS_MODULE,
-	.name           = "ibmasmfs",
-	.init_fs_context = ibmasmfs_init_fs_context,
-	.kill_sb        = kill_litter_super,
+	.owner          	= THIS_MODULE,
+	.name           	= "ibmasmfs",
+	.init_fs_context	= ibmasmfs_init_fs_context,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 MODULE_ALIAS_FS("ibmasmfs");
 
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 6e9ef35a43a7ba..83eaed3f1a8e0b 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1602,9 +1602,8 @@ static int ffs_fs_init_fs_context(struct fs_context *fc)
 }
 
 static void
-ffs_fs_kill_sb(struct super_block *sb)
+ffs_fs_free_sb(struct super_block *sb)
 {
-	kill_litter_super(sb);
 	if (sb->s_fs_info)
 		ffs_data_closed(sb->s_fs_info);
 }
@@ -1614,7 +1613,8 @@ static struct file_system_type ffs_fs_type = {
 	.name		= "functionfs",
 	.init_fs_context = ffs_fs_init_fs_context,
 	.parameters	= ffs_fs_fs_parameters,
-	.kill_sb	= ffs_fs_kill_sb,
+	.shutdown_sb	= litter_shutdown_sb,
+	.free_sb	= ffs_fs_free_sb,
 };
 MODULE_ALIAS_FS("functionfs");
 
diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index a203266bc0dc82..e79e907b0a065e 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -2086,9 +2086,8 @@ static int gadgetfs_init_fs_context(struct fs_context *fc)
 }
 
 static void
-gadgetfs_kill_sb (struct super_block *sb)
+gadgetfs_free_sb(struct super_block *sb)
 {
-	kill_litter_super (sb);
 	if (the_device) {
 		put_dev (the_device);
 		the_device = NULL;
@@ -2100,10 +2099,11 @@ gadgetfs_kill_sb (struct super_block *sb)
 /*----------------------------------------------------------------------*/
 
 static struct file_system_type gadgetfs_type = {
-	.owner		= THIS_MODULE,
-	.name		= shortname,
-	.init_fs_context = gadgetfs_init_fs_context,
-	.kill_sb	= gadgetfs_kill_sb,
+	.owner			= THIS_MODULE,
+	.name			= shortname,
+	.init_fs_context	= gadgetfs_init_fs_context,
+	.shutdown_sb		= litter_shutdown_sb,
+	.free_sb		= gadgetfs_free_sb,
 };
 MODULE_ALIAS_FS("gadgetfs");
 
diff --git a/drivers/xen/xenfs/super.c b/drivers/xen/xenfs/super.c
index d7d64235010d41..ab2e3d3d05bb2e 100644
--- a/drivers/xen/xenfs/super.c
+++ b/drivers/xen/xenfs/super.c
@@ -85,10 +85,10 @@ static int xenfs_init_fs_context(struct fs_context *fc)
 }
 
 static struct file_system_type xenfs_type = {
-	.owner =	THIS_MODULE,
-	.name =		"xenfs",
-	.init_fs_context = xenfs_init_fs_context,
-	.kill_sb =	kill_litter_super,
+	.owner			= THIS_MODULE,
+	.name			= "xenfs",
+	.init_fs_context	= xenfs_init_fs_context,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 MODULE_ALIAS_FS("xenfs");
 
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index e0108d17b085cf..0eb5bd91cfeafb 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -809,10 +809,10 @@ static struct linux_binfmt misc_format = {
 };
 
 static struct file_system_type bm_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "binfmt_misc",
-	.init_fs_context = bm_init_fs_context,
-	.kill_sb	= kill_litter_super,
+	.owner			= THIS_MODULE,
+	.name			= "binfmt_misc",
+	.init_fs_context	= bm_init_fs_context,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 MODULE_ALIAS_FS("binfmt_misc");
 
diff --git a/fs/configfs/mount.c b/fs/configfs/mount.c
index c2d820063ec49a..87043caf048a2c 100644
--- a/fs/configfs/mount.c
+++ b/fs/configfs/mount.c
@@ -112,10 +112,10 @@ static int configfs_init_fs_context(struct fs_context *fc)
 }
 
 static struct file_system_type configfs_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "configfs",
-	.init_fs_context = configfs_init_fs_context,
-	.kill_sb	= kill_litter_super,
+	.owner			= THIS_MODULE,
+	.name			= "configfs",
+	.init_fs_context	= configfs_init_fs_context,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 MODULE_ALIAS_FS("configfs");
 
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 83e57e9f9fa037..a4955c8f6638ae 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -300,10 +300,10 @@ static struct dentry *debug_mount(struct file_system_type *fs_type,
 }
 
 static struct file_system_type debug_fs_type = {
-	.owner =	THIS_MODULE,
-	.name =		"debugfs",
-	.mount =	debug_mount,
-	.kill_sb =	kill_litter_super,
+	.owner			= THIS_MODULE,
+	.name			= "debugfs",
+	.mount			= debug_mount,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 MODULE_ALIAS_FS("debugfs");
 
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index d46cea36c026ad..bc7ff574dbbf90 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -487,11 +487,10 @@ static struct dentry *devpts_mount(struct file_system_type *fs_type,
 	return mount_nodev(fs_type, flags, data, devpts_fill_super);
 }
 
-static void devpts_kill_sb(struct super_block *sb)
+static void devpts_free_sb(struct super_block *sb)
 {
 	struct pts_fs_info *fsi = DEVPTS_SB(sb);
 
-	kill_litter_super(sb);
 	if (fsi)
 		ida_destroy(&fsi->allocated_ptys);
 	kfree(fsi);
@@ -500,7 +499,8 @@ static void devpts_kill_sb(struct super_block *sb)
 static struct file_system_type devpts_fs_type = {
 	.name		= "devpts",
 	.mount		= devpts_mount,
-	.kill_sb	= devpts_kill_sb,
+	.shutdown_sb	= litter_shutdown_sb,
+	.free_sb	= devpts_free_sb,
 	.fs_flags	= FS_USERNS_MOUNT,
 };
 
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index e028fafa04f38c..f9e8df15c35067 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -279,10 +279,8 @@ static int efivarfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void efivarfs_kill_sb(struct super_block *sb)
+static void efivarfs_free_sb(struct super_block *sb)
 {
-	kill_litter_super(sb);
-
 	if (!efivar_is_available())
 		return;
 
@@ -291,10 +289,11 @@ static void efivarfs_kill_sb(struct super_block *sb)
 }
 
 static struct file_system_type efivarfs_type = {
-	.owner   = THIS_MODULE,
-	.name    = "efivarfs",
-	.init_fs_context = efivarfs_init_fs_context,
-	.kill_sb = efivarfs_kill_sb,
+	.owner   		= THIS_MODULE,
+	.name    		= "efivarfs",
+	.init_fs_context	= efivarfs_init_fs_context,
+	.shutdown_sb		= litter_shutdown_sb,
+	.free_sb		= efivarfs_free_sb,
 };
 
 static __init int efivarfs_init(void)
diff --git a/fs/fuse/control.c b/fs/fuse/control.c
index ab62e46242568a..3a24d00a165971 100644
--- a/fs/fuse/control.c
+++ b/fs/fuse/control.c
@@ -352,7 +352,7 @@ static int fuse_ctl_init_fs_context(struct fs_context *fsc)
 	return 0;
 }
 
-static void fuse_ctl_kill_sb(struct super_block *sb)
+static void fuse_ctl_shutdown_sb(struct super_block *sb)
 {
 	struct fuse_conn *fc;
 
@@ -362,14 +362,14 @@ static void fuse_ctl_kill_sb(struct super_block *sb)
 		fc->ctl_ndents = 0;
 	mutex_unlock(&fuse_mutex);
 
-	kill_litter_super(sb);
+	litter_shutdown_sb(sb);
 }
 
 static struct file_system_type fuse_ctl_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "fusectl",
-	.init_fs_context = fuse_ctl_init_fs_context,
-	.kill_sb	= fuse_ctl_kill_sb,
+	.owner			= THIS_MODULE,
+	.name			= "fusectl",
+	.init_fs_context	= fuse_ctl_init_fs_context,
+	.shutdown_sb		= fuse_ctl_shutdown_sb,
 };
 MODULE_ALIAS_FS("fusectl");
 
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 316c4cebd3f3de..1a722aba8c6645 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1588,7 +1588,7 @@ static struct file_system_type hugetlbfs_fs_type = {
 	.name			= "hugetlbfs",
 	.init_fs_context	= hugetlbfs_init_fs_context,
 	.parameters		= hugetlb_fs_parameters,
-	.kill_sb		= kill_litter_super,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 
 static struct vfsmount *hugetlbfs_vfsmount[HUGE_MAX_HSTATE];
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 7ed02fb88a362c..a2be86da4efa15 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1440,21 +1440,23 @@ static int nfsd_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void nfsd_umount(struct super_block *sb)
+static void nfsd_shutdown_sb(struct super_block *sb)
 {
-	struct net *net = sb->s_fs_info;
-
-	nfsd_shutdown_threads(net);
+	nfsd_shutdown_threads(sb->s_fs_info);
+	litter_shutdown_sb(sb);
+}
 
-	kill_litter_super(sb);
-	put_net(net);
+static void nfsd_free_sb(struct super_block *sb)
+{
+	put_net(sb->s_fs_info);
 }
 
 static struct file_system_type nfsd_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "nfsd",
-	.init_fs_context = nfsd_init_fs_context,
-	.kill_sb	= nfsd_umount,
+	.owner			= THIS_MODULE,
+	.name			= "nfsd",
+	.init_fs_context	= nfsd_init_fs_context,
+	.shutdown_sb		= nfsd_shutdown_sb,
+	.free_sb		= nfsd_free_sb,
 };
 MODULE_ALIAS_FS("nfsd");
 
diff --git a/fs/ocfs2/dlmfs/dlmfs.c b/fs/ocfs2/dlmfs/dlmfs.c
index 81265123ce6ce5..b987653f6ec070 100644
--- a/fs/ocfs2/dlmfs/dlmfs.c
+++ b/fs/ocfs2/dlmfs/dlmfs.c
@@ -567,7 +567,7 @@ static struct file_system_type dlmfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "ocfs2_dlmfs",
 	.mount		= dlmfs_mount,
-	.kill_sb	= kill_litter_super,
+	.shutdown_sb	= litter_shutdown_sb,
 };
 MODULE_ALIAS_FS("ocfs2_dlmfs");
 
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index fd1d24b47160d0..36635ed77b6873 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -465,10 +465,8 @@ static struct dentry *pstore_mount(struct file_system_type *fs_type,
 	return mount_single(fs_type, flags, data, pstore_fill_super);
 }
 
-static void pstore_kill_sb(struct super_block *sb)
+static void pstore_free_sb(struct super_block *sb)
 {
-	kill_litter_super(sb);
-
 	mutex_lock(&pstore_sb_lock);
 	pstore_sb = NULL;
 
@@ -483,7 +481,8 @@ static struct file_system_type pstore_fs_type = {
 	.owner          = THIS_MODULE,
 	.name		= "pstore",
 	.mount		= pstore_mount,
-	.kill_sb	= pstore_kill_sb,
+	.shutdown_sb	= litter_shutdown_sb,
+	.free_sb	= pstore_free_sb,
 };
 
 int __init pstore_init_fs(void)
diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
index 0f37ecbae59dad..51eade68ae06f8 100644
--- a/fs/ramfs/inode.c
+++ b/fs/ramfs/inode.c
@@ -278,9 +278,8 @@ int ramfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-void ramfs_kill_sb(struct super_block *sb)
+void ramfs_free_sb(struct super_block *sb)
 {
-	kill_litter_super(sb);
 	kfree(sb->s_fs_info);
 }
 
@@ -288,7 +287,8 @@ static struct file_system_type ramfs_fs_type = {
 	.name		= "ramfs",
 	.init_fs_context = ramfs_init_fs_context,
 	.parameters	= ramfs_fs_parameters,
-	.kill_sb	= ramfs_kill_sb,
+	.shutdown_sb	= litter_shutdown_sb,
+	.free_sb	= ramfs_free_sb,
 	.fs_flags	= FS_USERNS_MOUNT,
 };
 
diff --git a/fs/super.c b/fs/super.c
index 8e173eccc8c113..1173a272bd086a 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1319,13 +1319,21 @@ void free_anon_bdev(dev_t dev)
 }
 EXPORT_SYMBOL(free_anon_bdev);
 
-void kill_litter_super(struct super_block *sb)
+/**
+ * litter_shutdown_sb - shut down a super_block that only has synthetic files
+ * @sb:		super_block to shut down
+ *
+ * This function is a drop in ->shutdown_sb method and calls d_genocide() to
+ * drop all entries in the dcache.  It is used for file systems that only
+ * contained synthetic, that is kernel-generated, entries created by helpers
+ * like simple_fill_super().
+ */
+void litter_shutdown_sb(struct super_block *sb)
 {
 	if (sb->s_root)
 		d_genocide(sb->s_root);
-	generic_shutdown_super(sb);
 }
-EXPORT_SYMBOL(kill_litter_super);
+EXPORT_SYMBOL(litter_shutdown_sb);
 
 static int test_keyed_super(struct super_block *sb, struct fs_context *fc)
 {
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index de5b72216b1a70..7cde49dd76c5f7 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -438,7 +438,7 @@ static struct file_system_type trace_fs_type = {
 	.owner =	THIS_MODULE,
 	.name =		"tracefs",
 	.mount =	trace_mount,
-	.kill_sb =	kill_litter_super,
+	.shutdown_sb =	litter_shutdown_sb,
 };
 MODULE_ALIAS_FS("tracefs");
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 12fff7df3cc46b..c44c6fe9fc045b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2384,7 +2384,7 @@ extern struct dentry *mount_subtree(struct vfsmount *mnt, const char *path);
 void retire_super(struct super_block *sb);
 void generic_shutdown_super(struct super_block *sb);
 void kill_block_super(struct super_block *sb);
-void kill_litter_super(struct super_block *sb);
+void litter_shutdown_sb(struct super_block *sb);
 void deactivate_super(struct super_block *sb);
 void deactivate_locked_super(struct super_block *sb);
 int get_anon_bdev(dev_t *);
diff --git a/include/linux/ramfs.h b/include/linux/ramfs.h
index d506dc63dd47c5..9065f3bc8c9e94 100644
--- a/include/linux/ramfs.h
+++ b/include/linux/ramfs.h
@@ -7,7 +7,7 @@
 struct inode *ramfs_get_inode(struct super_block *sb, const struct inode *dir,
 	 umode_t mode, dev_t dev);
 extern int ramfs_init_fs_context(struct fs_context *fc);
-extern void ramfs_kill_sb(struct super_block *sb);
+void ramfs_free_sb(struct super_block *sb);
 
 #ifdef CONFIG_MMU
 static inline int
diff --git a/init/do_mounts.c b/init/do_mounts.c
index 5dfd30b13f4857..95f40b8784d9aa 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -503,9 +503,9 @@ static int rootfs_init_fs_context(struct fs_context *fc)
 }
 
 struct file_system_type rootfs_fs_type = {
-	.name		= "rootfs",
-	.init_fs_context = rootfs_init_fs_context,
-	.kill_sb	= kill_litter_super,
+	.name			= "rootfs",
+	.init_fs_context	= rootfs_init_fs_context,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 
 void __init init_rootfs(void)
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index ba8215ed663a43..e2e2fa6ffb901c 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -1681,7 +1681,7 @@ static const struct fs_context_operations mqueue_fs_context_ops = {
 static struct file_system_type mqueue_fs_type = {
 	.name			= "mqueue",
 	.init_fs_context	= mqueue_init_fs_context,
-	.kill_sb		= kill_litter_super,
+	.shutdown_sb		= litter_shutdown_sb,
 	.fs_flags		= FS_USERNS_MOUNT,
 };
 
diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index 99d0625b6c828f..877a2a6a5729ba 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -797,7 +797,7 @@ static struct file_system_type bpf_fs_type = {
 	.name		= "bpf",
 	.init_fs_context = bpf_init_fs_context,
 	.parameters	= bpf_fs_parameters,
-	.kill_sb	= kill_litter_super,
+	.shutdown_sb	= litter_shutdown_sb,
 };
 
 static int __init bpf_init(void)
diff --git a/mm/shmem.c b/mm/shmem.c
index 02e62fccc80d49..53a39134e863f3 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -4584,7 +4584,7 @@ static struct file_system_type shmem_fs_type = {
 #ifdef CONFIG_TMPFS
 	.parameters	= shmem_fs_parameters,
 #endif
-	.kill_sb	= kill_litter_super,
+	.shutdown_sb	= litter_shutdown_sb,
 #ifdef CONFIG_SHMEM
 	.fs_flags	= FS_USERNS_MOUNT | FS_ALLOW_IDMAP | FS_MGTIME,
 #else
@@ -4709,7 +4709,8 @@ static struct file_system_type shmem_fs_type = {
 	.name		= "tmpfs",
 	.init_fs_context = ramfs_init_fs_context,
 	.parameters	= ramfs_fs_parameters,
-	.kill_sb	= ramfs_kill_sb,
+	.shutdown_sb	= litter_shutdown_sb,
+	.free_sb	= ramfs_free_sb,
 	.fs_flags	= FS_USERNS_MOUNT,
 };
 
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index f420d845734513..c285cbe1a821a8 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -1441,7 +1441,7 @@ static int rpc_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void rpc_kill_sb(struct super_block *sb)
+static void rpc_shutdown_sb(struct super_block *sb)
 {
 	struct net *net = sb->s_fs_info;
 	struct sunrpc_net *sn = net_generic(net, sunrpc_net_id);
@@ -1459,15 +1459,20 @@ static void rpc_kill_sb(struct super_block *sb)
 					   sb);
 	mutex_unlock(&sn->pipefs_sb_lock);
 out:
-	kill_litter_super(sb);
-	put_net(net);
+	litter_shutdown_sb(sb);
+}
+
+static void rpc_free_sb(struct super_block *sb)
+{
+	put_net(sb->s_fs_info);
 }
 
 static struct file_system_type rpc_pipe_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "rpc_pipefs",
-	.init_fs_context = rpc_init_fs_context,
-	.kill_sb	= rpc_kill_sb,
+	.owner			= THIS_MODULE,
+	.name			= "rpc_pipefs",
+	.init_fs_context	= rpc_init_fs_context,
+	.shutdown_sb		= rpc_shutdown_sb,
+	.free_sb		= rpc_free_sb,
 };
 MODULE_ALIAS_FS("rpc_pipefs");
 MODULE_ALIAS("rpc_pipefs");
diff --git a/security/inode.c b/security/inode.c
index 3aa75fffa8c929..84779f998e76c5 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -67,10 +67,10 @@ static int securityfs_init_fs_context(struct fs_context *fc)
 }
 
 static struct file_system_type fs_type = {
-	.owner =	THIS_MODULE,
-	.name =		"securityfs",
-	.init_fs_context = securityfs_init_fs_context,
-	.kill_sb =	kill_litter_super,
+	.owner			= THIS_MODULE,
+	.name			= "securityfs",
+	.init_fs_context	= securityfs_init_fs_context,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 
 /**
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 08fbb0f89d2659..e619eedefca373 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -94,7 +94,7 @@ static int selinux_fs_info_create(struct super_block *sb)
 	return 0;
 }
 
-static void selinux_fs_info_free(struct super_block *sb)
+static void sel_free_sb(struct super_block *sb)
 {
 	struct selinux_fs_info *fsi = sb->s_fs_info;
 	unsigned int i;
@@ -2120,16 +2120,11 @@ static int sel_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void sel_kill_sb(struct super_block *sb)
-{
-	kill_litter_super(sb);
-	selinux_fs_info_free(sb);
-}
-
 static struct file_system_type sel_fs_type = {
-	.name		= "selinuxfs",
-	.init_fs_context = sel_init_fs_context,
-	.kill_sb	= sel_kill_sb,
+	.name			= "selinuxfs",
+	.init_fs_context	= sel_init_fs_context,
+	.shutdown_sb		= litter_shutdown_sb,
+	.free_sb		= sel_free_sb,
 };
 
 static struct vfsmount *selinuxfs_mount __ro_after_init;
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index e22aad7604e8ac..9e60e927d02deb 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -2979,9 +2979,9 @@ static int smk_init_fs_context(struct fs_context *fc)
 }
 
 static struct file_system_type smk_fs_type = {
-	.name		= "smackfs",
-	.init_fs_context = smk_init_fs_context,
-	.kill_sb	= kill_litter_super,
+	.name			= "smackfs",
+	.init_fs_context	= smk_init_fs_context,
+	.shutdown_sb		= litter_shutdown_sb,
 };
 
 static struct vfsmount *smackfs_mount;
-- 
2.39.2

Re: [PATCH 12/19] fs: convert kill_litter_super to litter_shutdown_sb

[Kees Cook]

On Wed, Sep 13, 2023 at 08:10:06AM -0300, Christoph Hellwig wrote:
> Replace kill_litter_super with litter_shutdown_sb, which is wired up to
> the ->shutdown_sb method.  For file systems that wrapped
> kill_litter_super, ->kill_sb is replaced with ->shutdown and ->free_sb
> methods as needed.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Kees Cook <keescook@chromium.org> # for pstore

-- 
Kees Cook

[PATCH 13/19] fs: convert kill_block_super to block_free_sb

[Christoph Hellwig]

Replace kill_block_super with block_free_sb, which is wired up to
the ->free_sb method.  For file systems that wrapped kill_block_super,
->kill_sb is replaced with ->shutdown and ->free_sb methods as needed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/adfs/super.c          |  2 +-
 fs/affs/super.c          |  7 ++++---
 fs/befs/linuxvfs.c       |  2 +-
 fs/bfs/inode.c           |  2 +-
 fs/efs/super.c           |  7 ++++---
 fs/erofs/super.c         | 25 ++++++++++---------------
 fs/exfat/super.c         |  6 +++---
 fs/ext2/super.c          |  2 +-
 fs/ext4/super.c          | 12 ++++++------
 fs/f2fs/super.c          |  6 +++---
 fs/fat/namei_msdos.c     |  2 +-
 fs/fat/namei_vfat.c      |  2 +-
 fs/freevxfs/vxfs_super.c |  2 +-
 fs/fuse/inode.c          | 12 ++++++------
 fs/gfs2/ops_fstype.c     | 11 ++++-------
 fs/hfs/super.c           |  2 +-
 fs/hfsplus/super.c       |  2 +-
 fs/hpfs/super.c          |  2 +-
 fs/isofs/inode.c         |  2 +-
 fs/jfs/super.c           |  2 +-
 fs/minix/inode.c         |  2 +-
 fs/nilfs2/super.c        |  2 +-
 fs/ntfs/super.c          |  2 +-
 fs/ntfs3/super.c         |  6 +++---
 fs/ocfs2/super.c         |  2 +-
 fs/omfs/inode.c          |  2 +-
 fs/qnx4/inode.c          |  7 ++++---
 fs/qnx6/inode.c          |  2 +-
 fs/reiserfs/super.c      |  7 +++----
 fs/squashfs/super.c      |  2 +-
 fs/super.c               |  6 ++----
 fs/sysv/super.c          |  4 ++--
 fs/udf/super.c           |  2 +-
 fs/ufs/super.c           |  2 +-
 fs/xfs/xfs_buf.c         |  2 +-
 fs/xfs/xfs_super.c       |  6 +++---
 fs/zonefs/super.c        | 13 ++++++-------
 include/linux/fs.h       |  2 +-
 38 files changed, 86 insertions(+), 95 deletions(-)

diff --git a/fs/adfs/super.c b/fs/adfs/super.c
index e8bfc38239cd59..22f0137f485e5f 100644
--- a/fs/adfs/super.c
+++ b/fs/adfs/super.c
@@ -463,7 +463,7 @@ static struct file_system_type adfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "adfs",
 	.mount		= adfs_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("adfs");
diff --git a/fs/affs/super.c b/fs/affs/super.c
index 58b391446ae1fd..775e878797f9fc 100644
--- a/fs/affs/super.c
+++ b/fs/affs/super.c
@@ -631,10 +631,11 @@ static struct dentry *affs_mount(struct file_system_type *fs_type,
 	return mount_bdev(fs_type, flags, dev_name, data, affs_fill_super);
 }
 
-static void affs_kill_sb(struct super_block *sb)
+static void affs_free_sb(struct super_block *sb)
 {
 	struct affs_sb_info *sbi = AFFS_SB(sb);
-	kill_block_super(sb);
+
+	block_free_sb(sb);
 	if (sbi) {
 		affs_free_bitmap(sb);
 		affs_brelse(sbi->s_root_bh);
@@ -648,7 +649,7 @@ static struct file_system_type affs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "affs",
 	.mount		= affs_mount,
-	.kill_sb	= affs_kill_sb,
+	.free_sb	= affs_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("affs");
diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index 9a16a51fbb88d4..7682c027d44782 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -981,7 +981,7 @@ static struct file_system_type befs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "befs",
 	.mount		= befs_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("befs");
diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c
index e6a76ae9eb4442..4d894d5dd07074 100644
--- a/fs/bfs/inode.c
+++ b/fs/bfs/inode.c
@@ -458,7 +458,7 @@ static struct file_system_type bfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "bfs",
 	.mount		= bfs_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("bfs");
diff --git a/fs/efs/super.c b/fs/efs/super.c
index b287f47c165ba8..1f808a455e7e87 100644
--- a/fs/efs/super.c
+++ b/fs/efs/super.c
@@ -28,10 +28,11 @@ static struct dentry *efs_mount(struct file_system_type *fs_type,
 	return mount_bdev(fs_type, flags, dev_name, data, efs_fill_super);
 }
 
-static void efs_kill_sb(struct super_block *s)
+static void efs_free_sb(struct super_block *s)
 {
 	struct efs_sb_info *sbi = SUPER_INFO(s);
-	kill_block_super(s);
+
+	block_free_sb(s);
 	kfree(sbi);
 }
 
@@ -39,7 +40,7 @@ static struct file_system_type efs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "efs",
 	.mount		= efs_mount,
-	.kill_sb	= efs_kill_sb,
+	.free_sb	= efs_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("efs");
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 07c36ccf454e53..29b87bb35b1ddc 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -872,22 +872,17 @@ static int erofs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void erofs_kill_sb(struct super_block *sb)
+static void erofs_free_sb(struct super_block *sb)
 {
-	struct erofs_sb_info *sbi;
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
 
 	/* pseudo mount for anon inodes */
-	if (sb->s_flags & SB_KERNMOUNT) {
-		generic_shutdown_super(sb);
+	if (sb->s_flags & SB_KERNMOUNT)
 		return;
-	}
 
-	if (erofs_is_fscache_mode(sb))
-		generic_shutdown_super(sb);
-	else
-		kill_block_super(sb);
+	if (!erofs_is_fscache_mode(sb))
+		block_free_sb(sb);
 
-	sbi = EROFS_SB(sb);
 	if (!sbi)
 		return;
 
@@ -921,11 +916,11 @@ static void erofs_put_super(struct super_block *sb)
 }
 
 struct file_system_type erofs_fs_type = {
-	.owner          = THIS_MODULE,
-	.name           = "erofs",
-	.init_fs_context = erofs_init_fs_context,
-	.kill_sb        = erofs_kill_sb,
-	.fs_flags       = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
+	.owner          	= THIS_MODULE,
+	.name           	= "erofs",
+	.init_fs_context	= erofs_init_fs_context,
+	.free_sb		= erofs_free_sb,
+	.fs_flags       	= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
 };
 MODULE_ALIAS_FS("erofs");
 
diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index 2778bd9b631e72..c040f964f3ca75 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -764,11 +764,11 @@ static int exfat_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void exfat_kill_sb(struct super_block *sb)
+static void exfat_free_sb(struct super_block *sb)
 {
 	struct exfat_sb_info *sbi = sb->s_fs_info;
 
-	kill_block_super(sb);
+	block_free_sb(sb);
 	if (sbi)
 		exfat_free_sbi(sbi);
 }
@@ -778,7 +778,7 @@ static struct file_system_type exfat_fs_type = {
 	.name			= "exfat",
 	.init_fs_context	= exfat_init_fs_context,
 	.parameters		= exfat_parameters,
-	.kill_sb		= exfat_kill_sb,
+	.free_sb		= exfat_free_sb,
 	.fs_flags		= FS_REQUIRES_DEV,
 };
 
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index aaf3e3e88cb218..80bbf5b9009732 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1628,7 +1628,7 @@ static struct file_system_type ext2_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "ext2",
 	.mount		= ext2_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("ext2");
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 38217422f93883..24e125e2da19ca 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -93,7 +93,7 @@ static int ext4_get_tree(struct fs_context *fc);
 static int ext4_reconfigure(struct fs_context *fc);
 static void ext4_fc_free(struct fs_context *fc);
 static int ext4_init_fs_context(struct fs_context *fc);
-static void ext4_kill_sb(struct super_block *sb);
+static void ext4_free_sb(struct super_block *sb);
 static const struct fs_parameter_spec ext4_param_specs[];
 
 /*
@@ -136,7 +136,7 @@ static struct file_system_type ext2_fs_type = {
 	.name			= "ext2",
 	.init_fs_context	= ext4_init_fs_context,
 	.parameters		= ext4_param_specs,
-	.kill_sb		= ext4_kill_sb,
+	.free_sb		= ext4_free_sb,
 	.fs_flags		= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("ext2");
@@ -152,7 +152,7 @@ static struct file_system_type ext3_fs_type = {
 	.name			= "ext3",
 	.init_fs_context	= ext4_init_fs_context,
 	.parameters		= ext4_param_specs,
-	.kill_sb		= ext4_kill_sb,
+	.free_sb		= ext4_free_sb,
 	.fs_flags		= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("ext3");
@@ -7297,12 +7297,12 @@ static inline int ext3_feature_set_ok(struct super_block *sb)
 	return 1;
 }
 
-static void ext4_kill_sb(struct super_block *sb)
+static void ext4_free_sb(struct super_block *sb)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct block_device *journal_bdev = sbi ? sbi->s_journal_bdev : NULL;
 
-	kill_block_super(sb);
+	block_free_sb(sb);
 
 	if (journal_bdev)
 		blkdev_put(journal_bdev, sb);
@@ -7313,7 +7313,7 @@ static struct file_system_type ext4_fs_type = {
 	.name			= "ext4",
 	.init_fs_context	= ext4_init_fs_context,
 	.parameters		= ext4_param_specs,
-	.kill_sb		= ext4_kill_sb,
+	.free_sb		= ext4_free_sb,
 	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP | FS_MGTIME,
 };
 MODULE_ALIAS_FS("ext4");
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a8c8232852bb18..571eb08683d0ea 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4850,7 +4850,7 @@ static struct dentry *f2fs_mount(struct file_system_type *fs_type, int flags,
 	return mount_bdev(fs_type, flags, dev_name, data, f2fs_fill_super);
 }
 
-static void kill_f2fs_super(struct super_block *sb)
+static void f2fs_shutdown_sb(struct super_block *sb)
 {
 	if (sb->s_root) {
 		struct f2fs_sb_info *sbi = F2FS_SB(sb);
@@ -4880,14 +4880,14 @@ static void kill_f2fs_super(struct super_block *sb)
 		if (is_sbi_flag_set(sbi, SBI_IS_RECOVERED) && f2fs_readonly(sb))
 			sb->s_flags &= ~SB_RDONLY;
 	}
-	kill_block_super(sb);
 }
 
 static struct file_system_type f2fs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "f2fs",
 	.mount		= f2fs_mount,
-	.kill_sb	= kill_f2fs_super,
+	.shutdown_sb	= f2fs_shutdown_sb,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
 };
 MODULE_ALIAS_FS("f2fs");
diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
index 2116c486843b7d..429e417c964a7a 100644
--- a/fs/fat/namei_msdos.c
+++ b/fs/fat/namei_msdos.c
@@ -666,7 +666,7 @@ static struct file_system_type msdos_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "msdos",
 	.mount		= msdos_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
 };
 MODULE_ALIAS_FS("msdos");
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index c4d00999a43300..4b0500c4d4c554 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -1211,7 +1211,7 @@ static struct file_system_type vfat_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "vfat",
 	.mount		= vfat_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
 };
 MODULE_ALIAS_FS("vfat");
diff --git a/fs/freevxfs/vxfs_super.c b/fs/freevxfs/vxfs_super.c
index 310d73e254df2c..d33f3dae0663ff 100644
--- a/fs/freevxfs/vxfs_super.c
+++ b/fs/freevxfs/vxfs_super.c
@@ -292,7 +292,7 @@ static struct file_system_type vxfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "vxfs",
 	.mount		= vxfs_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("vxfs"); /* makes mount -t vxfs autoload the module */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 5731003b56a9c9..d9981532530702 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1818,7 +1818,7 @@ void fuse_conn_destroy(struct fuse_mount *fm)
 }
 EXPORT_SYMBOL_GPL(fuse_conn_destroy);
 
-static void fuse_sb_destroy(struct super_block *sb)
+static void fuse_shutdown_sb(struct super_block *sb)
 {
 	struct fuse_mount *fm = get_fuse_mount_super(sb);
 	bool last;
@@ -1839,7 +1839,7 @@ EXPORT_SYMBOL(fuse_mount_destroy);
 
 static void fuse_kill_sb_anon(struct super_block *sb)
 {
-	fuse_sb_destroy(sb);
+	fuse_shutdown_sb(sb);
 	generic_shutdown_super(sb);
 	fuse_mount_destroy(get_fuse_mount_super(sb));
 }
@@ -1855,10 +1855,9 @@ static struct file_system_type fuse_fs_type = {
 MODULE_ALIAS_FS("fuse");
 
 #ifdef CONFIG_BLOCK
-static void fuse_kill_sb_blk(struct super_block *sb)
+static void fuseblk_free_sb(struct super_block *sb)
 {
-	fuse_sb_destroy(sb);
-	kill_block_super(sb);
+	block_free_sb(sb);
 	fuse_mount_destroy(get_fuse_mount_super(sb));
 }
 
@@ -1867,7 +1866,8 @@ static struct file_system_type fuseblk_fs_type = {
 	.name		= "fuseblk",
 	.init_fs_context = fuse_init_fs_context,
 	.parameters	= fuse_fs_parameters,
-	.kill_sb	= fuse_kill_sb_blk,
+	.shutdown_sb	= fuse_shutdown_sb,
+	.free_sb	= fuseblk_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV | FS_HAS_SUBTYPE,
 };
 MODULE_ALIAS_FS("fuseblk");
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 33ca04733e933e..2c84ec7dd5b3d5 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1781,14 +1781,12 @@ static void gfs2_evict_inodes(struct super_block *sb)
 	iput(toput_inode);
 }
 
-static void gfs2_kill_sb(struct super_block *sb)
+static void gfs2_shutdown_sb(struct super_block *sb)
 {
 	struct gfs2_sbd *sdp = sb->s_fs_info;
 
-	if (sdp == NULL) {
-		kill_block_super(sb);
+	if (!sdp)
 		return;
-	}
 
 	gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_SYNC | GFS2_LFC_KILL_SB);
 	dput(sdp->sd_root_dir);
@@ -1807,8 +1805,6 @@ static void gfs2_kill_sb(struct super_block *sb)
 	set_bit(SDF_KILL, &sdp->sd_flags);
 	gfs2_flush_delete_work(sdp);
 	destroy_workqueue(sdp->sd_delete_wq);
-
-	kill_block_super(sb);
 }
 
 struct file_system_type gfs2_fs_type = {
@@ -1816,7 +1812,8 @@ struct file_system_type gfs2_fs_type = {
 	.fs_flags = FS_REQUIRES_DEV,
 	.init_fs_context = gfs2_init_fs_context,
 	.parameters = gfs2_fs_parameters,
-	.kill_sb = gfs2_kill_sb,
+	.shutdown_sb = gfs2_shutdown_sb,
+	.free_sb = block_free_sb,
 	.owner = THIS_MODULE,
 };
 MODULE_ALIAS_FS("gfs2");
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 6764afa98a6ff1..56dbce9d4daa3d 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -460,7 +460,7 @@ static struct file_system_type hfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "hfs",
 	.mount		= hfs_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("hfs");
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 1986b4f18a9013..d0994cdeafdb17 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -645,7 +645,7 @@ static struct file_system_type hfsplus_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "hfsplus",
 	.mount		= hfsplus_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("hfsplus");
diff --git a/fs/hpfs/super.c b/fs/hpfs/super.c
index 758a51564124dc..df8a641fe71657 100644
--- a/fs/hpfs/super.c
+++ b/fs/hpfs/super.c
@@ -763,7 +763,7 @@ static struct file_system_type hpfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "hpfs",
 	.mount		= hpfs_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("hpfs");
diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index 2ee21286ac8f07..524fae2ff0d9bf 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -1563,7 +1563,7 @@ static struct file_system_type iso9660_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "iso9660",
 	.mount		= isofs_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("iso9660");
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index 2e2f7f6d36a09d..6a44da0c652194 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -905,7 +905,7 @@ static struct file_system_type jfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "jfs",
 	.mount		= jfs_do_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("jfs");
diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index df575473c1cc0b..9e812e0882bbb1 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -688,7 +688,7 @@ static struct file_system_type minix_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "minix",
 	.mount		= minix_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("minix");
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index a5d1fa4e7552f6..95b0e33a665d8c 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -1370,7 +1370,7 @@ struct file_system_type nilfs_fs_type = {
 	.owner    = THIS_MODULE,
 	.name     = "nilfs2",
 	.mount    = nilfs_mount,
-	.kill_sb  = kill_block_super,
+	.free_sb  = block_free_sb,
 	.fs_flags = FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("nilfs2");
diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 56a7d5bd33e4e2..811283d709faeb 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -3061,7 +3061,7 @@ static struct file_system_type ntfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "ntfs",
 	.mount		= ntfs_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("ntfs");
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index cfec5e0c7f66ae..8e8b614773e071 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -1720,11 +1720,11 @@ static int ntfs_init_fs_context(struct fs_context *fc)
 	return -ENOMEM;
 }
 
-static void ntfs3_kill_sb(struct super_block *sb)
+static void ntfs3_free_sb(struct super_block *sb)
 {
 	struct ntfs_sb_info *sbi = sb->s_fs_info;
 
-	kill_block_super(sb);
+	block_free_sb(sb);
 
 	if (sbi->options)
 		put_mount_options(sbi->options);
@@ -1737,7 +1737,7 @@ static struct file_system_type ntfs_fs_type = {
 	.name			= "ntfs3",
 	.init_fs_context	= ntfs_init_fs_context,
 	.parameters		= ntfs_fs_parameters,
-	.kill_sb		= ntfs3_kill_sb,
+	.free_sb		= ntfs3_free_sb,
 	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
 };
 // clang-format on
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 6b906424902b46..fc63858a6744af 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -1190,7 +1190,7 @@ static struct file_system_type ocfs2_fs_type = {
 	.owner          = THIS_MODULE,
 	.name           = "ocfs2",
 	.mount          = ocfs2_mount,
-	.kill_sb        = kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags       = FS_REQUIRES_DEV|FS_RENAME_DOES_D_MOVE,
 	.next           = NULL
 };
diff --git a/fs/omfs/inode.c b/fs/omfs/inode.c
index 2f8c1882f45c85..7bb65fccc7a6b0 100644
--- a/fs/omfs/inode.c
+++ b/fs/omfs/inode.c
@@ -606,7 +606,7 @@ static struct file_system_type omfs_fs_type = {
 	.owner = THIS_MODULE,
 	.name = "omfs",
 	.mount = omfs_mount,
-	.kill_sb = kill_block_super,
+	.free_sb = block_free_sb,
 	.fs_flags = FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("omfs");
diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
index a7171f5532a1f8..56825072ac90da 100644
--- a/fs/qnx4/inode.c
+++ b/fs/qnx4/inode.c
@@ -235,10 +235,11 @@ static int qnx4_fill_super(struct super_block *s, void *data, int silent)
 	return 0;
 }
 
-static void qnx4_kill_sb(struct super_block *sb)
+static void qnx4_free_sb(struct super_block *sb)
 {
 	struct qnx4_sb_info *qs = qnx4_sb(sb);
-	kill_block_super(sb);
+
+	block_free_sb(sb);
 	if (qs) {
 		kfree(qs->BitMap);
 		kfree(qs);
@@ -388,7 +389,7 @@ static struct file_system_type qnx4_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "qnx4",
 	.mount		= qnx4_mount,
-	.kill_sb	= qnx4_kill_sb,
+	.free_sb	= qnx4_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("qnx4");
diff --git a/fs/qnx6/inode.c b/fs/qnx6/inode.c
index 21f90d519f1a16..1ac37a6bdec7dc 100644
--- a/fs/qnx6/inode.c
+++ b/fs/qnx6/inode.c
@@ -644,7 +644,7 @@ static struct file_system_type qnx6_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "qnx6",
 	.mount		= qnx6_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("qnx6");
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 7eaf36b3de12b4..e9633533d20261 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -547,7 +547,7 @@ int remove_save_link(struct inode *inode, int truncate)
 	return journal_end(&th);
 }
 
-static void reiserfs_kill_sb(struct super_block *s)
+static void reiserfs_shutdown_sb(struct super_block *s)
 {
 	if (REISERFS_SB(s)) {
 		reiserfs_proc_info_done(s);
@@ -566,8 +566,6 @@ static void reiserfs_kill_sb(struct super_block *s)
 		dput(REISERFS_SB(s)->priv_root);
 		REISERFS_SB(s)->priv_root = NULL;
 	}
-
-	kill_block_super(s);
 }
 
 #ifdef CONFIG_QUOTA
@@ -2634,7 +2632,8 @@ struct file_system_type reiserfs_fs_type = {
 	.owner = THIS_MODULE,
 	.name = "reiserfs",
 	.mount = get_super_block,
-	.kill_sb = reiserfs_kill_sb,
+	.shutdown_sb = reiserfs_shutdown_sb,
+	.free_sb = block_free_sb,
 	.fs_flags = FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("reiserfs");
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index 22e812808e5cf9..ecb9d3a22a859c 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -678,7 +678,7 @@ static struct file_system_type squashfs_fs_type = {
 	.name = "squashfs",
 	.init_fs_context = squashfs_init_fs_context,
 	.parameters = squashfs_fs_parameters,
-	.kill_sb = kill_block_super,
+	.free_sb = block_free_sb,
 	.fs_flags = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
 };
 MODULE_ALIAS_FS("squashfs");
diff --git a/fs/super.c b/fs/super.c
index 1173a272bd086a..805ca1dd1e23f2 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1665,18 +1665,16 @@ struct dentry *mount_bdev(struct file_system_type *fs_type,
 }
 EXPORT_SYMBOL(mount_bdev);
 
-void kill_block_super(struct super_block *sb)
+void block_free_sb(struct super_block *sb)
 {
 	struct block_device *bdev = sb->s_bdev;
 
-	generic_shutdown_super(sb);
 	if (bdev) {
 		sync_blockdev(bdev);
 		blkdev_put(bdev, sb);
 	}
 }
-
-EXPORT_SYMBOL(kill_block_super);
+EXPORT_SYMBOL(block_free_sb);
 #endif
 
 struct dentry *mount_nodev(struct file_system_type *fs_type,
diff --git a/fs/sysv/super.c b/fs/sysv/super.c
index 3365a30dc1e0cd..028a813af3a520 100644
--- a/fs/sysv/super.c
+++ b/fs/sysv/super.c
@@ -544,7 +544,7 @@ static struct file_system_type sysv_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "sysv",
 	.mount		= sysv_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("sysv");
@@ -553,7 +553,7 @@ static struct file_system_type v7_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "v7",
 	.mount		= v7_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("v7");
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 928a04d9d9e0ad..c610cf41e4efae 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -129,7 +129,7 @@ static struct file_system_type udf_fstype = {
 	.owner		= THIS_MODULE,
 	.name		= "udf",
 	.mount		= udf_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("udf");
diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 23377c1baed9e0..ab3e9f4b657824 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -1512,7 +1512,7 @@ static struct file_system_type ufs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "ufs",
 	.mount		= ufs_mount,
-	.kill_sb	= kill_block_super,
+	.free_sb	= block_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("ufs");
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index c1ece4a08ff446..bf2e43d6885bfc 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1953,7 +1953,7 @@ xfs_free_buftarg(
 	list_lru_destroy(&btp->bt_lru);
 
 	fs_put_dax(btp->bt_daxdev, btp->bt_mount);
-	/* the main block device is closed by kill_block_super */
+	/* the main block device is closed by block_free_sb */
 	if (bdev != btp->bt_mount->m_super->s_bdev)
 		blkdev_put(bdev, btp->bt_mount->m_super);
 
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 1f77014c6e1abd..a3c07ca4e648c2 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -2052,10 +2052,10 @@ static int xfs_init_fs_context(
 }
 
 static void
-xfs_kill_sb(
+xfs_free_sb(
 	struct super_block		*sb)
 {
-	kill_block_super(sb);
+	block_free_sb(sb);
 	xfs_mount_free(XFS_M(sb));
 }
 
@@ -2064,7 +2064,7 @@ static struct file_system_type xfs_fs_type = {
 	.name			= "xfs",
 	.init_fs_context	= xfs_init_fs_context,
 	.parameters		= xfs_fs_parameters,
-	.kill_sb		= xfs_kill_sb,
+	.free_sb		= xfs_free_sb,
 	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP | FS_MGTIME,
 };
 MODULE_ALIAS_FS("xfs");
diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
index 35b2554ce2ac2e..0406e0e33ebdbc 100644
--- a/fs/zonefs/super.c
+++ b/fs/zonefs/super.c
@@ -1221,7 +1221,7 @@ static int zonefs_get_zgroup_inodes(struct super_block *sb)
 	return 0;
 }
 
-static void zonefs_release_zgroup_inodes(struct super_block *sb)
+static void zonefs_shutdown_sb(struct super_block *sb)
 {
 	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
 	enum zonefs_ztype ztype;
@@ -1229,6 +1229,7 @@ static void zonefs_release_zgroup_inodes(struct super_block *sb)
 	if (!sbi)
 		return;
 
+	/* Release the reference on the zone group directory inodes */
 	for (ztype = 0; ztype < ZONEFS_ZTYPE_MAX; ztype++) {
 		if (sbi->s_zgroup[ztype].g_inode) {
 			iput(sbi->s_zgroup[ztype].g_inode);
@@ -1351,14 +1352,11 @@ static struct dentry *zonefs_mount(struct file_system_type *fs_type,
 	return mount_bdev(fs_type, flags, dev_name, data, zonefs_fill_super);
 }
 
-static void zonefs_kill_super(struct super_block *sb)
+static void zonefs_free_sb(struct super_block *sb)
 {
 	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
 
-	/* Release the reference on the zone group directory inodes */
-	zonefs_release_zgroup_inodes(sb);
-
-	kill_block_super(sb);
+	block_free_sb(sb);
 
 	zonefs_sysfs_unregister(sb);
 	zonefs_free_zgroups(sb);
@@ -1372,7 +1370,8 @@ static struct file_system_type zonefs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "zonefs",
 	.mount		= zonefs_mount,
-	.kill_sb	= zonefs_kill_super,
+	.shutdown_sb	= zonefs_shutdown_sb,
+	.free_sb	= zonefs_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c44c6fe9fc045b..302be5dfc1a04a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2383,7 +2383,7 @@ extern struct dentry *mount_nodev(struct file_system_type *fs_type,
 extern struct dentry *mount_subtree(struct vfsmount *mnt, const char *path);
 void retire_super(struct super_block *sb);
 void generic_shutdown_super(struct super_block *sb);
-void kill_block_super(struct super_block *sb);
+void block_free_sb(struct super_block *sb);
 void litter_shutdown_sb(struct super_block *sb);
 void deactivate_super(struct super_block *sb);
 void deactivate_locked_super(struct super_block *sb);
-- 
2.39.2

Re: [PATCH 13/19] fs: convert kill_block_super to block_free_sb

[Al Viro]

On Wed, Sep 13, 2023 at 08:10:07AM -0300, Christoph Hellwig wrote:

> -static void affs_kill_sb(struct super_block *sb)
> +static void affs_free_sb(struct super_block *sb)
>  {
>  	struct affs_sb_info *sbi = AFFS_SB(sb);
> -	kill_block_super(sb);
> +
> +	block_free_sb(sb);
>  	if (sbi) {
>  		affs_free_bitmap(sb);
>  		affs_brelse(sbi->s_root_bh);

<checks>

Yep, that's printk + brelse()...  Could we have that
block_free_sb() (an awful name aside) done after the
if (sbi) { ... } there?

[PATCH 14/19] jffs2: convert to ->shutdown_sb and ->free_sb

[Christoph Hellwig]

Convert jffs2 from ->kill_sb to ->shutdown_sb and ->free_sb.  Drop
the otherwise unused kill_mtd_super helpers, as there is no benefit in
it over just calling put_mtd_device on sb->s_mtd.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/mtd/mtdsuper.c    | 12 ------------
 fs/jffs2/super.c          | 22 ++++++++++++++--------
 include/linux/mtd/super.h |  2 --
 3 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c
index b7e3763c47f0cd..66da2e6f90f5f5 100644
--- a/drivers/mtd/mtdsuper.c
+++ b/drivers/mtd/mtdsuper.c
@@ -165,15 +165,3 @@ int get_tree_mtd(struct fs_context *fc,
 	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(get_tree_mtd);
-
-/*
- * destroy an MTD-based superblock
- */
-void kill_mtd_super(struct super_block *sb)
-{
-	generic_shutdown_super(sb);
-	put_mtd_device(sb->s_mtd);
-	sb->s_mtd = NULL;
-}
-
-EXPORT_SYMBOL_GPL(kill_mtd_super);
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 7ea37f49f1e18e..14577368202e90 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -340,21 +340,27 @@ static void jffs2_put_super (struct super_block *sb)
 	jffs2_dbg(1, "%s(): returning\n", __func__);
 }
 
-static void jffs2_kill_sb(struct super_block *sb)
+static void jffs2_shutdown_sb(struct super_block *sb)
 {
 	struct jffs2_sb_info *c = JFFS2_SB_INFO(sb);
+
 	if (c && !sb_rdonly(sb))
 		jffs2_stop_garbage_collect_thread(c);
-	kill_mtd_super(sb);
-	kfree(c);
+}
+
+static void jffs2_free_sb(struct super_block *sb)
+{
+	put_mtd_device(sb->s_mtd);
+	kfree(JFFS2_SB_INFO(sb));
 }
 
 static struct file_system_type jffs2_fs_type = {
-	.owner =	THIS_MODULE,
-	.name =		"jffs2",
-	.init_fs_context = jffs2_init_fs_context,
-	.parameters =	jffs2_fs_parameters,
-	.kill_sb =	jffs2_kill_sb,
+	.owner			= THIS_MODULE,
+	.name			= "jffs2",
+	.init_fs_context	= jffs2_init_fs_context,
+	.parameters		= jffs2_fs_parameters,
+	.shutdown_sb		= jffs2_shutdown_sb,
+	.free_sb		= jffs2_free_sb,
 };
 MODULE_ALIAS_FS("jffs2");
 
diff --git a/include/linux/mtd/super.h b/include/linux/mtd/super.h
index 3608a6c36faceb..f6d5c1a17eec23 100644
--- a/include/linux/mtd/super.h
+++ b/include/linux/mtd/super.h
@@ -17,8 +17,6 @@
 extern int get_tree_mtd(struct fs_context *fc,
 		     int (*fill_super)(struct super_block *sb,
 				       struct fs_context *fc));
-extern void kill_mtd_super(struct super_block *sb);
-
 
 #endif /* __KERNEL__ */
 
-- 
2.39.2

[PATCH 15/19] kernfs: split ->kill_sb

[Christoph Hellwig]

Split the kernfs_kill_sb helper into helpers for the new split
shutdown_sb and free_sb methods.  Note that resctrl has very odd
locking in ->kill_sb, so this commit only releases the locking
acquired in rdt_shutdown_sb in rdt_free_sb.  This is not very good
code and relies on ->shutdown_sb and ->free_sb to always be called
in pairs, which it currently is.  The next commit will try to clean
this up.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 12 +++++++++---
 fs/kernfs/mount.c                      | 18 ++++++++----------
 fs/sysfs/mount.c                       |  7 ++++---
 include/linux/kernfs.h                 |  5 ++---
 kernel/cgroup/cgroup.c                 | 10 ++++++----
 5 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 725344048f85da..8db767fd80df6b 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2772,7 +2772,7 @@ static void rmdir_all_sub(void)
 	kernfs_remove(kn_mondata);
 }
 
-static void rdt_kill_sb(struct super_block *sb)
+static void rdt_shutdown_sb(struct super_block *sb)
 {
 	struct rdt_resource *r;
 
@@ -2792,7 +2792,12 @@ static void rdt_kill_sb(struct super_block *sb)
 	static_branch_disable_cpuslocked(&rdt_alloc_enable_key);
 	static_branch_disable_cpuslocked(&rdt_mon_enable_key);
 	static_branch_disable_cpuslocked(&rdt_enable_key);
-	kernfs_kill_sb(sb);
+	kernfs_shutdown_sb(sb);
+}
+
+static void rdt_free_sb(struct super_block *sb)
+{
+	kernfs_free_sb(sb);
 	mutex_unlock(&rdtgroup_mutex);
 	cpus_read_unlock();
 }
@@ -2801,7 +2806,8 @@ static struct file_system_type rdt_fs_type = {
 	.name			= "resctrl",
 	.init_fs_context	= rdt_init_fs_context,
 	.parameters		= rdt_fs_parameters,
-	.kill_sb		= rdt_kill_sb,
+	.shutdown_sb		= rdt_shutdown_sb,
+	.free_sb		= rdt_free_sb,
 };
 
 static int mon_addfile(struct kernfs_node *parent_kn, const char *name,
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index d6d3cba669dbdd..32ec4ec3c878f6 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -379,14 +379,14 @@ void kernfs_free_fs_context(struct fs_context *fc)
 }
 
 /**
- * kernfs_kill_sb - kill_sb for kernfs
+ * kernfs_shutdown_sb - shutdown_sb for kernfs
  * @sb: super_block being killed
  *
- * This can be used directly for file_system_type->kill_sb().  If a kernfs
- * user needs extra cleanup, it can implement its own kill_sb() and call
+ * This can be used directly for file_system_type->shutdown_sb().  If a kernfs
+ * user needs extra cleanup, it can implement its own shutdown_sb() and call
  * this function at the end.
  */
-void kernfs_kill_sb(struct super_block *sb)
+void kernfs_shutdown_sb(struct super_block *sb)
 {
 	struct kernfs_super_info *info = kernfs_info(sb);
 	struct kernfs_root *root = info->root;
@@ -394,13 +394,11 @@ void kernfs_kill_sb(struct super_block *sb)
 	down_write(&root->kernfs_supers_rwsem);
 	list_del(&info->node);
 	up_write(&root->kernfs_supers_rwsem);
+}
 
-	/*
-	 * Remove the superblock from fs_supers/s_instances
-	 * so we can't find it, before freeing kernfs_super_info.
-	 */
-	generic_shutdown_super(sb);
-	kfree(info);
+void kernfs_free_sb(struct super_block *sb)
+{
+	kfree(kernfs_info(sb));
 }
 
 static void __init kernfs_mutex_init(void)
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index 98467bb7673781..804391342599bc 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -79,18 +79,19 @@ static int sysfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void sysfs_kill_sb(struct super_block *sb)
+static void sysfs_free_sb(struct super_block *sb)
 {
 	void *ns = (void *)kernfs_super_ns(sb);
 
-	kernfs_kill_sb(sb);
+	kernfs_free_sb(sb);
 	kobj_ns_drop(KOBJ_NS_TYPE_NET, ns);
 }
 
 static struct file_system_type sysfs_fs_type = {
 	.name			= "sysfs",
 	.init_fs_context	= sysfs_init_fs_context,
-	.kill_sb		= sysfs_kill_sb,
+	.shutdown_sb		= kernfs_shutdown_sb,
+	.free_sb		= sysfs_free_sb,
 	.fs_flags		= FS_USERNS_MOUNT,
 };
 
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 2a36f3218b5106..940059251deac8 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -453,7 +453,8 @@ int kernfs_xattr_set(struct kernfs_node *kn, const char *name,
 const void *kernfs_super_ns(struct super_block *sb);
 int kernfs_get_tree(struct fs_context *fc);
 void kernfs_free_fs_context(struct fs_context *fc);
-void kernfs_kill_sb(struct super_block *sb);
+void kernfs_shutdown_sb(struct super_block *sb);
+void kernfs_free_sb(struct super_block *sb);
 
 void kernfs_init(void);
 
@@ -572,8 +573,6 @@ static inline int kernfs_get_tree(struct fs_context *fc)
 
 static inline void kernfs_free_fs_context(struct fs_context *fc) { }
 
-static inline void kernfs_kill_sb(struct super_block *sb) { }
-
 static inline void kernfs_init(void) { }
 
 #endif	/* CONFIG_KERNFS */
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 1fb7f562289d53..a6c5e6ed1b6e2d 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2249,7 +2249,7 @@ static int cgroup_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void cgroup_kill_sb(struct super_block *sb)
+static void cgroup_shutdown_sb(struct super_block *sb)
 {
 	struct kernfs_root *kf_root = kernfs_root_from_sb(sb);
 	struct cgroup_root *root = cgroup_root_from_kf(kf_root);
@@ -2266,14 +2266,15 @@ static void cgroup_kill_sb(struct super_block *sb)
 		percpu_ref_kill(&root->cgrp.self.refcnt);
 	}
 	cgroup_put(&root->cgrp);
-	kernfs_kill_sb(sb);
+	kernfs_shutdown_sb(sb);
 }
 
 struct file_system_type cgroup_fs_type = {
 	.name			= "cgroup",
 	.init_fs_context	= cgroup_init_fs_context,
 	.parameters		= cgroup1_fs_parameters,
-	.kill_sb		= cgroup_kill_sb,
+	.shutdown_sb		= cgroup_shutdown_sb,
+	.free_sb		= kernfs_free_sb,
 	.fs_flags		= FS_USERNS_MOUNT,
 };
 
@@ -2281,7 +2282,8 @@ static struct file_system_type cgroup2_fs_type = {
 	.name			= "cgroup2",
 	.init_fs_context	= cgroup_init_fs_context,
 	.parameters		= cgroup2_fs_parameters,
-	.kill_sb		= cgroup_kill_sb,
+	.shutdown_sb		= cgroup_shutdown_sb,
+	.free_sb		= kernfs_free_sb,
 	.fs_flags		= FS_USERNS_MOUNT,
 };
 
-- 
2.39.2

Re: [PATCH 15/19] kernfs: split ->kill_sb

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 1117 bytes --]

On Wed, Sep 13, 2023 at 08:10:09AM -0300, Christoph Hellwig <hch@lst.de> wrote:
> Split the kernfs_kill_sb helper into helpers for the new split
> shutdown_sb and free_sb methods.  Note that resctrl has very odd
> locking in ->kill_sb, so this commit only releases the locking
> acquired in rdt_shutdown_sb in rdt_free_sb.  This is not very good
> code and relies on ->shutdown_sb and ->free_sb to always be called
> in pairs, which it currently is.  The next commit will try to clean
> this up.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 12 +++++++++---
>  fs/kernfs/mount.c                      | 18 ++++++++----------
>  fs/sysfs/mount.c                       |  7 ++++---
>  include/linux/kernfs.h                 |  5 ++---
>  kernel/cgroup/cgroup.c                 | 10 ++++++----
>  5 files changed, 29 insertions(+), 23 deletions(-)

Reviewed-by: Michal Koutný <mkoutny@suse.com>

(Also, I didn't find a necessity to have kernfs_free_sb(sb) under
rdtgroup_mutex, so folding the following patch of the series may be fine
too.)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 16/19] x86/resctrl: release rdtgroup_mutex and the CPU hotplug lock in rdt_shutdown_sb

[Christoph Hellwig]

While the resctl code is a bit confusing, I can't find anything protected
by rdtgroup_mutex or the CPU hotplug lock in generic_shutdown_super or
kernfs_free_sb.  Drop the locks at the end of rdt_shutdown_sb to avoid
holding locks over method calls and VFS code which itself already has a
rather complicated locking hierarchy.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 8db767fd80df6b..e87de519493021 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2793,11 +2793,6 @@ static void rdt_shutdown_sb(struct super_block *sb)
 	static_branch_disable_cpuslocked(&rdt_mon_enable_key);
 	static_branch_disable_cpuslocked(&rdt_enable_key);
 	kernfs_shutdown_sb(sb);
-}
-
-static void rdt_free_sb(struct super_block *sb)
-{
-	kernfs_free_sb(sb);
 	mutex_unlock(&rdtgroup_mutex);
 	cpus_read_unlock();
 }
@@ -2807,7 +2802,7 @@ static struct file_system_type rdt_fs_type = {
 	.init_fs_context	= rdt_init_fs_context,
 	.parameters		= rdt_fs_parameters,
 	.shutdown_sb		= rdt_shutdown_sb,
-	.free_sb		= rdt_free_sb,
+	.free_sb		= kernfs_free_sb,
 };
 
 static int mon_addfile(struct kernfs_node *parent_kn, const char *name,
-- 
2.39.2

[PATCH 17/19] NFS: move nfs_kill_super to fs_context.c

[Christoph Hellwig]

nfs_kill_super is only used in fs_context, so move it there.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/fs_context.c | 13 +++++++++++++
 fs/nfs/internal.h   |  1 -
 fs/nfs/super.c      | 16 ----------------
 fs/nfs/sysfs.h      |  2 ++
 4 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 853e8d609bb3bc..ee82e4cfb38bb5 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -21,8 +21,10 @@
 
 #include <net/handshake.h>
 
+#include "fscache.h"
 #include "nfs.h"
 #include "internal.h"
+#include "sysfs.h"
 
 #include "nfstrace.h"
 
@@ -1644,6 +1646,17 @@ static int nfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
+static void nfs_kill_super(struct super_block *s)
+{
+	struct nfs_server *server = NFS_SB(s);
+
+	nfs_sysfs_move_sb_to_server(server);
+	generic_shutdown_super(s);
+
+	nfs_fscache_release_super_cookie(s);
+	nfs_free_server(server);
+}
+
 struct file_system_type nfs_fs_type = {
 	.owner			= THIS_MODULE,
 	.name			= "nfs",
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 9c9cf764f6000d..49d5b03176c02d 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -447,7 +447,6 @@ extern const struct super_operations nfs_sops;
 bool nfs_auth_info_match(const struct nfs_auth_info *, rpc_authflavor_t);
 int nfs_try_get_tree(struct fs_context *);
 int nfs_get_tree_common(struct fs_context *);
-void nfs_kill_super(struct super_block *);
 
 extern struct rpc_stat nfs_rpcstat;
 
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 89131e855e1393..5ba793e7f262d4 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1329,22 +1329,6 @@ int nfs_get_tree_common(struct fs_context *fc)
 	goto out;
 }
 
-/*
- * Destroy an NFS superblock
- */
-void nfs_kill_super(struct super_block *s)
-{
-	struct nfs_server *server = NFS_SB(s);
-
-	nfs_sysfs_move_sb_to_server(server);
-	generic_shutdown_super(s);
-
-	nfs_fscache_release_super_cookie(s);
-
-	nfs_free_server(server);
-}
-EXPORT_SYMBOL_GPL(nfs_kill_super);
-
 #if IS_ENABLED(CONFIG_NFS_V4)
 
 /*
diff --git a/fs/nfs/sysfs.h b/fs/nfs/sysfs.h
index c5d1990cade50a..44c8a1712149c2 100644
--- a/fs/nfs/sysfs.h
+++ b/fs/nfs/sysfs.h
@@ -8,6 +8,8 @@
 
 #define CONTAINER_ID_MAXLEN (64)
 
+struct nfs_net;
+
 struct nfs_netns_client {
 	struct kobject kobject;
 	struct kobject nfs_net_kobj;
-- 
2.39.2

[PATCH 18/19] fs: simple ->shutdown_sb and ->free_sb conversions

[Christoph Hellwig]

Convert all file systems that just called generic_shutdown_super from
->kill_sb without any state kept from before the call to after it to
->shutdown_sb and ->free_sb as needed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/9p/vfs_super.c             | 14 +++-----------
 fs/afs/super.c                | 15 +++++++++++----
 fs/autofs/autofs_i.h          |  3 ++-
 fs/autofs/init.c              |  3 ++-
 fs/autofs/inode.c             | 24 +++++++++++++-----------
 fs/btrfs/super.c              | 11 ++++-------
 fs/ceph/super.c               | 13 +++++++++----
 fs/cramfs/inode.c             |  6 ++----
 fs/ecryptfs/main.c            | 11 ++---------
 fs/fuse/inode.c               |  7 +++----
 fs/fuse/virtio_fs.c           | 19 +++++++++++--------
 fs/hostfs/hostfs_kern.c       |  5 ++---
 fs/nfs/fs_context.c           | 18 ++++++++++--------
 fs/orangefs/orangefs-kernel.h |  2 +-
 fs/orangefs/orangefs-mod.c    |  2 +-
 fs/orangefs/super.c           | 11 ++++-------
 fs/proc/root.c                | 16 +++++++++-------
 fs/romfs/super.c              |  6 ++----
 fs/smb/client/cifsfs.c        | 14 +++++++++-----
 fs/ubifs/super.c              |  9 +++------
 20 files changed, 103 insertions(+), 106 deletions(-)

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index e8b3641c98f886..a238065dd8b361 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -205,25 +205,17 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
 	return ERR_PTR(retval);
 }
 
-/**
- * v9fs_kill_super - Kill Superblock
- * @s: superblock
- *
- */
-
-static void v9fs_kill_super(struct super_block *s)
+static void v9fs_free_sb(struct super_block *s)
 {
 	struct v9fs_session_info *v9ses = s->s_fs_info;
 
 	p9_debug(P9_DEBUG_VFS, " %p\n", s);
 
-	generic_shutdown_super(s);
-
 	v9fs_session_cancel(v9ses);
 	v9fs_session_close(v9ses);
 	kfree(v9ses);
 	s->s_fs_info = NULL;
-	p9_debug(P9_DEBUG_VFS, "exiting kill_super\n");
+	p9_debug(P9_DEBUG_VFS, "exiting free_sb\n");
 }
 
 static void
@@ -340,7 +332,7 @@ static const struct super_operations v9fs_super_ops_dotl = {
 struct file_system_type v9fs_fs_type = {
 	.name = "9p",
 	.mount = v9fs_mount,
-	.kill_sb = v9fs_kill_super,
+	.free_sb = v9fs_free_sb,
 	.owner = THIS_MODULE,
 	.fs_flags = FS_RENAME_DOES_D_MOVE,
 };
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 84b135ad3496b1..bd85554056415d 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -30,7 +30,8 @@
 #include "internal.h"
 
 static void afs_i_init_once(void *foo);
-static void afs_kill_super(struct super_block *sb);
+static void afs_shutdown_sb(struct super_block *sb);
+static void afs_free_sb(struct super_block *sb);
 static struct inode *afs_alloc_inode(struct super_block *sb);
 static void afs_destroy_inode(struct inode *inode);
 static void afs_free_inode(struct inode *inode);
@@ -45,7 +46,8 @@ struct file_system_type afs_fs_type = {
 	.name			= "afs",
 	.init_fs_context	= afs_init_fs_context,
 	.parameters		= afs_fs_parameters,
-	.kill_sb		= afs_kill_super,
+	.shutdown_sb		= afs_shutdown_sb,
+	.free_sb		= afs_free_sb,
 	.fs_flags		= FS_RENAME_DOES_D_MOVE,
 };
 MODULE_ALIAS_FS("afs");
@@ -527,7 +529,7 @@ static void afs_destroy_sbi(struct afs_super_info *as)
 	}
 }
 
-static void afs_kill_super(struct super_block *sb)
+static void afs_shutdown_sb(struct super_block *sb)
 {
 	struct afs_super_info *as = AFS_FS_S(sb);
 
@@ -539,7 +541,12 @@ static void afs_kill_super(struct super_block *sb)
 	 */
 	if (as->volume)
 		rcu_assign_pointer(as->volume->sb, NULL);
-	generic_shutdown_super(sb);
+}
+
+static void afs_free_sb(struct super_block *sb)
+{
+	struct afs_super_info *as = AFS_FS_S(sb);
+
 	if (as->volume)
 		afs_deactivate_volume(as->volume);
 	afs_destroy_sbi(as);
diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index d5a44fa88acf9a..f60f425c08b55c 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -276,4 +276,5 @@ static inline void autofs_del_expiring(struct dentry *dentry)
 	}
 }
 
-void autofs_kill_sb(struct super_block *);
+void autofs_shutdown_sb(struct super_block *sb);
+void autofs_free_sb(struct super_block *sb);
diff --git a/fs/autofs/init.c b/fs/autofs/init.c
index d3f55e87433890..1f7bed5391f822 100644
--- a/fs/autofs/init.c
+++ b/fs/autofs/init.c
@@ -17,7 +17,8 @@ struct file_system_type autofs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "autofs",
 	.mount		= autofs_mount,
-	.kill_sb	= autofs_kill_sb,
+	.shutdown_sb	= autofs_shutdown_sb,
+	.free_sb	= autofs_free_sb,
 };
 MODULE_ALIAS_FS("autofs");
 MODULE_ALIAS("autofs");
diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index c3b64799155840..363c55f0ae6a6f 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -37,24 +37,26 @@ void autofs_free_ino(struct autofs_info *ino)
 	kfree_rcu(ino, rcu);
 }
 
-void autofs_kill_sb(struct super_block *sb)
+void autofs_shutdown_sb(struct super_block *sb)
 {
 	struct autofs_sb_info *sbi = autofs_sbi(sb);
 
 	/*
 	 * In the event of a failure in get_sb_nodev the superblock
-	 * info is not present so nothing else has been setup, so
-	 * just call generic_shutdown_super when we are called from
-	 * deactivate_super.
+	 * info is not present so nothing else has been setup.
 	 */
-	if (sbi) {
-		/* Free wait queues, close pipe */
-		autofs_catatonic_mode(sbi);
-		put_pid(sbi->oz_pgrp);
-	}
+	if (!sbi)
+		return;
+
+	/* Free wait queues, close pipe */
+	autofs_catatonic_mode(sbi);
+	put_pid(sbi->oz_pgrp);
+}
+
+void autofs_free_sb(struct super_block *sb)
+{
+	struct autofs_sb_info *sbi = autofs_sbi(sb);
 
-	pr_debug("shutting down\n");
-	generic_shutdown_super(sb);
 	if (sbi)
 		kfree_rcu(sbi, rcu);
 }
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 063b9aa313c227..9205f8919faeeb 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2136,19 +2136,16 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-static void btrfs_kill_super(struct super_block *sb)
+static void btrfs_free_sb(struct super_block *sb)
 {
-	struct btrfs_fs_info *fs_info = btrfs_sb(sb);
-
-	generic_shutdown_super(sb);
-	btrfs_free_fs_info(fs_info);
+	btrfs_free_fs_info(btrfs_sb(sb));
 }
 
 static struct file_system_type btrfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "btrfs",
 	.mount		= btrfs_mount,
-	.kill_sb	= btrfs_kill_super,
+	.free_sb	= btrfs_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA | FS_MGTIME,
 };
 
@@ -2156,7 +2153,7 @@ static struct file_system_type btrfs_root_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "btrfs",
 	.mount		= btrfs_mount_root,
-	.kill_sb	= btrfs_kill_super,
+	.free_sb	= btrfs_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA |
 			  FS_ALLOW_IDMAP | FS_MGTIME,
 };
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index cbeaab8c21d8e6..327180889bb6e3 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1509,13 +1509,13 @@ void ceph_dec_osd_stopping_blocker(struct ceph_mds_client *mdsc)
 	__dec_stopping_blocker(mdsc);
 }
 
-static void ceph_kill_sb(struct super_block *s)
+static void ceph_shutdown_sb(struct super_block *s)
 {
 	struct ceph_fs_client *fsc = ceph_sb_to_client(s);
 	struct ceph_mds_client *mdsc = fsc->mdsc;
 	bool wait;
 
-	dout("kill_sb %p\n", s);
+	dout("shutdown_sb %p\n", s);
 
 	ceph_mdsc_pre_umount(mdsc);
 	flush_fs_workqueues(fsc);
@@ -1552,7 +1552,11 @@ static void ceph_kill_sb(struct super_block *s)
 	}
 
 	mdsc->stopping = CEPH_MDSC_STOPPING_FLUSHED;
-	generic_shutdown_super(s);
+}
+
+static void ceph_free_sb(struct super_block *s)
+{
+	struct ceph_fs_client *fsc = ceph_sb_to_client(s);
 
 	fsc->client->extra_mon_dispatch = NULL;
 	ceph_fs_debugfs_cleanup(fsc);
@@ -1566,7 +1570,8 @@ static struct file_system_type ceph_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "ceph",
 	.init_fs_context = ceph_init_fs_context,
-	.kill_sb	= ceph_kill_sb,
+	.shutdown_sb	= ceph_shutdown_sb,
+	.free_sb	= ceph_free_sb,
 	.fs_flags	= FS_RENAME_DOES_D_MOVE,
 };
 MODULE_ALIAS_FS("ceph");
diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
index 5ee7d7bbb361ce..afd05c3b1b5032 100644
--- a/fs/cramfs/inode.c
+++ b/fs/cramfs/inode.c
@@ -482,12 +482,10 @@ static const struct file_operations cramfs_physmem_fops = {
 #endif
 };
 
-static void cramfs_kill_sb(struct super_block *sb)
+static void cramfs_free_sb(struct super_block *sb)
 {
 	struct cramfs_sb_info *sbi = CRAMFS_SB(sb);
 
-	generic_shutdown_super(sb);
-
 	if (IS_ENABLED(CONFIG_CRAMFS_MTD) && sb->s_mtd) {
 		if (sbi && sbi->mtd_point_size)
 			mtd_unpoint(sb->s_mtd, 0, sbi->mtd_point_size);
@@ -977,7 +975,7 @@ static struct file_system_type cramfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "cramfs",
 	.init_fs_context = cramfs_init_fs_context,
-	.kill_sb	= cramfs_kill_sb,
+	.free_sb	= cramfs_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("cramfs");
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 3ed91537a3991a..cad39d6cc5d9d6 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -616,17 +616,10 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
 	return ERR_PTR(rc);
 }
 
-/**
- * ecryptfs_kill_block_super
- * @sb: The ecryptfs super block
- *
- * Used to bring the superblock down and free the private data.
- */
-static void ecryptfs_kill_block_super(struct super_block *sb)
+static void ecryptfs_free_sb(struct super_block *sb)
 {
 	struct ecryptfs_sb_info *sb_info = ecryptfs_superblock_to_private(sb);
 
-	generic_shutdown_super(sb);
 	if (!sb_info)
 		return;
 	ecryptfs_destroy_mount_crypt_stat(&sb_info->mount_crypt_stat);
@@ -637,7 +630,7 @@ static struct file_system_type ecryptfs_fs_type = {
 	.owner = THIS_MODULE,
 	.name = "ecryptfs",
 	.mount = ecryptfs_mount,
-	.kill_sb = ecryptfs_kill_block_super,
+	.free_sb = ecryptfs_free_sb,
 	.fs_flags = 0
 };
 MODULE_ALIAS_FS("ecryptfs");
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index d9981532530702..08d543b88b840d 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1837,10 +1837,8 @@ void fuse_mount_destroy(struct fuse_mount *fm)
 }
 EXPORT_SYMBOL(fuse_mount_destroy);
 
-static void fuse_kill_sb_anon(struct super_block *sb)
+static void fuse_free_sb_anon(struct super_block *sb)
 {
-	fuse_shutdown_sb(sb);
-	generic_shutdown_super(sb);
 	fuse_mount_destroy(get_fuse_mount_super(sb));
 }
 
@@ -1850,7 +1848,8 @@ static struct file_system_type fuse_fs_type = {
 	.fs_flags	= FS_HAS_SUBTYPE | FS_USERNS_MOUNT,
 	.init_fs_context = fuse_init_fs_context,
 	.parameters	= fuse_fs_parameters,
-	.kill_sb	= fuse_kill_sb_anon,
+	.shutdown_sb	= fuse_shutdown_sb,
+	.free_sb	= fuse_free_sb_anon,
 };
 MODULE_ALIAS_FS("fuse");
 
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index a52957df956394..8c2b0b226ddf63 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1387,19 +1387,21 @@ static void virtio_fs_conn_destroy(struct fuse_mount *fm)
 	virtio_fs_free_devs(vfs);
 }
 
-static void virtio_kill_sb(struct super_block *sb)
+static void virtio_shutdown_sb(struct super_block *sb)
 {
-	struct fuse_mount *fm = get_fuse_mount_super(sb);
-	bool last;
-
 	/* If mount failed, we can still be called without any fc */
 	if (sb->s_root) {
-		last = fuse_mount_remove(fm);
+		struct fuse_mount *fm = get_fuse_mount_super(sb);
+		bool last = fuse_mount_remove(fm);
+
 		if (last)
 			virtio_fs_conn_destroy(fm);
 	}
-	generic_shutdown_super(sb);
-	fuse_mount_destroy(fm);
+}
+
+static void virtio_free_sb(struct super_block *sb)
+{
+	fuse_mount_destroy(get_fuse_mount_super(sb));
 }
 
 static int virtio_fs_test_super(struct super_block *sb,
@@ -1507,7 +1509,8 @@ static struct file_system_type virtio_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "virtiofs",
 	.init_fs_context = virtio_fs_init_fs_context,
-	.kill_sb	= virtio_kill_sb,
+	.shutdown_sb	= virtio_shutdown_sb,
+	.free_sb	= virtio_free_sb,
 };
 
 static int __init virtio_fs_init(void)
diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
index 97f3c9709418c9..21d57098413222 100644
--- a/fs/hostfs/hostfs_kern.c
+++ b/fs/hostfs/hostfs_kern.c
@@ -979,9 +979,8 @@ static struct dentry *hostfs_read_sb(struct file_system_type *type,
 	return mount_nodev(type, flags, data, hostfs_fill_sb_common);
 }
 
-static void hostfs_kill_sb(struct super_block *s)
+static void hostfs_free_sb(struct super_block *s)
 {
-	generic_shutdown_super(s);
 	kfree(s->s_fs_info);
 }
 
@@ -989,7 +988,7 @@ static struct file_system_type hostfs_type = {
 	.owner 		= THIS_MODULE,
 	.name 		= "hostfs",
 	.mount	 	= hostfs_read_sb,
-	.kill_sb	= hostfs_kill_sb,
+	.free_sb	= hostfs_free_sb,
 	.fs_flags 	= 0,
 };
 MODULE_ALIAS_FS("hostfs");
diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index ee82e4cfb38bb5..4983931b5ad6e8 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -1646,15 +1646,15 @@ static int nfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void nfs_kill_super(struct super_block *s)
+static void nfs_shutdown_sb(struct super_block *s)
 {
-	struct nfs_server *server = NFS_SB(s);
-
-	nfs_sysfs_move_sb_to_server(server);
-	generic_shutdown_super(s);
+	nfs_sysfs_move_sb_to_server(NFS_SB(s));
+}
 
+static void nfs_free_sb(struct super_block *s)
+{
 	nfs_fscache_release_super_cookie(s);
-	nfs_free_server(server);
+	nfs_free_server(NFS_SB(s));
 }
 
 struct file_system_type nfs_fs_type = {
@@ -1662,7 +1662,8 @@ struct file_system_type nfs_fs_type = {
 	.name			= "nfs",
 	.init_fs_context	= nfs_init_fs_context,
 	.parameters		= nfs_fs_parameters,
-	.kill_sb		= nfs_kill_super,
+	.shutdown_sb		= nfs_shutdown_sb,
+	.free_sb		= nfs_free_sb,
 	.fs_flags		= FS_RENAME_DOES_D_MOVE|FS_BINARY_MOUNTDATA,
 };
 MODULE_ALIAS_FS("nfs");
@@ -1674,7 +1675,8 @@ struct file_system_type nfs4_fs_type = {
 	.name			= "nfs4",
 	.init_fs_context	= nfs_init_fs_context,
 	.parameters		= nfs_fs_parameters,
-	.kill_sb		= nfs_kill_super,
+	.shutdown_sb		= nfs_shutdown_sb,
+	.free_sb		= nfs_free_sb,
 	.fs_flags		= FS_RENAME_DOES_D_MOVE|FS_BINARY_MOUNTDATA,
 };
 MODULE_ALIAS_FS("nfs4");
diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
index b711654ca18a9c..73db7e6edb9966 100644
--- a/fs/orangefs/orangefs-kernel.h
+++ b/fs/orangefs/orangefs-kernel.h
@@ -344,7 +344,7 @@ struct dentry *orangefs_mount(struct file_system_type *fst,
 			   const char *devname,
 			   void *data);
 
-void orangefs_kill_sb(struct super_block *sb);
+void orangefs_free_sb(struct super_block *sb);
 int orangefs_remount(struct orangefs_sb_info_s *);
 
 int fsid_key_table_initialize(void);
diff --git a/fs/orangefs/orangefs-mod.c b/fs/orangefs/orangefs-mod.c
index 5ab741c60b7e29..8b87eab0828126 100644
--- a/fs/orangefs/orangefs-mod.c
+++ b/fs/orangefs/orangefs-mod.c
@@ -47,7 +47,7 @@ MODULE_PARM_DESC(hash_table_size,
 static struct file_system_type orangefs_fs_type = {
 	.name = "pvfs2",
 	.mount = orangefs_mount,
-	.kill_sb = orangefs_kill_sb,
+	.free_sb = orangefs_free_sb,
 	.owner = THIS_MODULE,
 };
 
diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index bf3a834ad15033..7c8d9a3fe34fca 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -580,9 +580,9 @@ struct dentry *orangefs_mount(struct file_system_type *fst,
 	return dget(sb->s_root);
 
 free_sb_and_op:
-	/* Will call orangefs_kill_sb with sb not in list. */
+	/* Will call orangefs_free_sb with sb not in list. */
 	ORANGEFS_SB(sb)->no_list = 1;
-	/* ORANGEFS_VFS_OP_FS_UMOUNT is done by orangefs_kill_sb. */
+	/* ORANGEFS_VFS_OP_FS_UMOUNT is done by orangefs_free_sb. */
 	deactivate_locked_super(sb);
 free_op:
 	gossip_err("orangefs_mount: mount request failed with %d\n", ret);
@@ -596,13 +596,10 @@ struct dentry *orangefs_mount(struct file_system_type *fst,
 	return d;
 }
 
-void orangefs_kill_sb(struct super_block *sb)
+void orangefs_free_sb(struct super_block *sb)
 {
 	int r;
-	gossip_debug(GOSSIP_SUPER_DEBUG, "orangefs_kill_sb: called\n");
-
-	/* provided sb cleanup */
-	generic_shutdown_super(sb);
+	gossip_debug(GOSSIP_SUPER_DEBUG, "orangefs_free_sb: called\n");
 
 	if (!ORANGEFS_SB(sb)) {
 		mutex_lock(&orangefs_request_mutex);
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 2282366449ac0b..81f6abedbd28b7 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -257,19 +257,20 @@ static int proc_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static void proc_kill_sb(struct super_block *sb)
+static void proc_shutdown_sb(struct super_block *sb)
 {
 	struct proc_fs_info *fs_info = proc_sb_info(sb);
 
-	if (!fs_info) {
-		generic_shutdown_super(sb);
+	if (!fs_info)
 		return;
-	}
-
 	dput(fs_info->proc_self);
 	dput(fs_info->proc_thread_self);
+}
+
+static void proc_free_sb(struct super_block *sb)
+{
+	struct proc_fs_info *fs_info = proc_sb_info(sb);
 
-	generic_shutdown_super(sb);
 	put_pid_ns(fs_info->pid_ns);
 	kfree(fs_info);
 }
@@ -278,7 +279,8 @@ static struct file_system_type proc_fs_type = {
 	.name			= "proc",
 	.init_fs_context	= proc_init_fs_context,
 	.parameters		= proc_fs_parameters,
-	.kill_sb		= proc_kill_sb,
+	.shutdown_sb		= proc_shutdown_sb,
+	.free_sb		= proc_free_sb,
 	.fs_flags		= FS_USERNS_MOUNT | FS_DISALLOW_NOTIFY_PERM,
 };
 
diff --git a/fs/romfs/super.c b/fs/romfs/super.c
index 5c35f6c760377e..d7c7e69db980e6 100644
--- a/fs/romfs/super.c
+++ b/fs/romfs/super.c
@@ -580,10 +580,8 @@ static int romfs_init_fs_context(struct fs_context *fc)
 /*
  * destroy a romfs superblock in the appropriate manner
  */
-static void romfs_kill_sb(struct super_block *sb)
+static void romfs_free_sb(struct super_block *sb)
 {
-	generic_shutdown_super(sb);
-
 #ifdef CONFIG_ROMFS_ON_MTD
 	if (sb->s_mtd) {
 		put_mtd_device(sb->s_mtd);
@@ -602,7 +600,7 @@ static struct file_system_type romfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "romfs",
 	.init_fs_context = romfs_init_fs_context,
-	.kill_sb	= romfs_kill_sb,
+	.free_sb	= romfs_free_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 };
 MODULE_ALIAS_FS("romfs");
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 5700f1065af756..34141c80a0bbd3 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -286,7 +286,7 @@ cifs_read_super(struct super_block *sb)
 	return rc;
 }
 
-static void cifs_kill_sb(struct super_block *sb)
+static void cifs_shutdown_sb(struct super_block *sb)
 {
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
 
@@ -301,9 +301,11 @@ static void cifs_kill_sb(struct super_block *sb)
 		dput(cifs_sb->root);
 		cifs_sb->root = NULL;
 	}
+}
 
-	generic_shutdown_super(sb);
-	cifs_umount(cifs_sb);
+static void cifs_free_sb(struct super_block *sb)
+{
+	cifs_umount(CIFS_SB(sb));
 }
 
 static int
@@ -1121,7 +1123,8 @@ struct file_system_type cifs_fs_type = {
 	.name = "cifs",
 	.init_fs_context = smb3_init_fs_context,
 	.parameters = smb3_fs_parameters,
-	.kill_sb = cifs_kill_sb,
+	.shutdown_sb = cifs_shutdown_sb,
+	.free_sb = cifs_free_sb,
 	.fs_flags = FS_RENAME_DOES_D_MOVE,
 };
 MODULE_ALIAS_FS("cifs");
@@ -1131,7 +1134,8 @@ struct file_system_type smb3_fs_type = {
 	.name = "smb3",
 	.init_fs_context = smb3_init_fs_context,
 	.parameters = smb3_fs_parameters,
-	.kill_sb = cifs_kill_sb,
+	.shutdown_sb = cifs_shutdown_sb,
+	.free_sb = cifs_free_sb,
 	.fs_flags = FS_RENAME_DOES_D_MOVE,
 };
 MODULE_ALIAS_FS("smb3");
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index b9d1ab63e3c87e..0bce555fd14d44 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2347,19 +2347,16 @@ static struct dentry *ubifs_mount(struct file_system_type *fs_type, int flags,
 	return ERR_PTR(err);
 }
 
-static void kill_ubifs_super(struct super_block *s)
+static void ubifs_free_sb(struct super_block *s)
 {
-	struct ubifs_info *c = s->s_fs_info;
-
-	generic_shutdown_super(s);
-	kfree(c);
+	kfree(s->s_fs_info);
 }
 
 static struct file_system_type ubifs_fs_type = {
 	.name    = "ubifs",
 	.owner   = THIS_MODULE,
 	.mount   = ubifs_mount,
-	.kill_sb = kill_ubifs_super,
+	.free_sb = ubifs_free_sb,
 };
 MODULE_ALIAS_FS("ubifs");
 
-- 
2.39.2

[PATCH 19/19] fs: remove ->kill_sb

[Christoph Hellwig]

Now that no instances are left, remove ->kill_sb and mark
generic_shutdown_super static.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/filesystems/locking.rst |  5 -----
 Documentation/filesystems/vfs.rst     |  5 -----
 fs/super.c                            | 25 +++++++++----------------
 include/linux/fs.h                    |  2 --
 4 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
index c33e2f03ed1f69..e4ca99c0828d00 100644
--- a/Documentation/filesystems/locking.rst
+++ b/Documentation/filesystems/locking.rst
@@ -221,7 +221,6 @@ prototypes::
 	struct dentry *(*mount) (struct file_system_type *, int,
 		       const char *, void *);
 	void (*shutdown_sb) (struct super_block *);
-	void (*kill_sb) (struct super_block *);
 	void (*free_sb) (struct super_block *);
 
 locking rules:
@@ -231,16 +230,12 @@ ops		may block
 =======		=========
 mount		yes
 shutdown_sb	yes
-kill_sb		yes
 free_sb		yes
 =======		=========
 
 ->mount() returns ERR_PTR or the root dentry; its superblock should be locked
 on return.
 
-->kill_sb() takes a write-locked superblock, does all shutdown work on it,
-unlocks and drops the reference.
-
 address_space_operations
 ========================
 prototypes::
diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst
index 1a7c6926c31f34..29513ee1d34ede 100644
--- a/Documentation/filesystems/vfs.rst
+++ b/Documentation/filesystems/vfs.rst
@@ -120,7 +120,6 @@ members are defined:
 		struct dentry *(*mount) (struct file_system_type *, int,
 			const char *, void *);
 		void (*shutdown_sb) (struct super_block *);
-		void (*kill_sb) (struct super_block *);
 		void (*free_sb) (struct super_block *);
 		struct module *owner;
 		struct file_system_type * next;
@@ -164,10 +163,6 @@ members are defined:
 	Note: dentries and inodes are normally taken care of and do not need
 	specific handling unless they are pinned by kernel users.
 
-``kill_sb``
-	the method to call when an instance of this filesystem should be
-	shut down
-
 ``free_sb``
 	Free file system specific resources like sb->s_fs_info that are
 	still needed while inodes are freed during umount.
diff --git a/fs/super.c b/fs/super.c
index 805ca1dd1e23f2..d9c564e70ffcd5 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -458,6 +458,8 @@ static void kill_super_notify(struct super_block *sb)
 	super_wake(sb, SB_DEAD);
 }
 
+static void generic_shutdown_super(struct super_block *sb);
+
 /**
  *	deactivate_locked_super	-	drop an active reference to superblock
  *	@s: superblock to deactivate
@@ -480,15 +482,11 @@ void deactivate_locked_super(struct super_block *s)
 
 	unregister_shrinker(&s->s_shrink);
 
-	if (fs->kill_sb) {
-		fs->kill_sb(s);
-	} else {
-		if (fs->shutdown_sb)
-			fs->shutdown_sb(s);
-		generic_shutdown_super(s);
-		if (fs->free_sb)
-			fs->free_sb(s);
-	}
+	if (fs->shutdown_sb)
+		fs->shutdown_sb(s);
+	generic_shutdown_super(s);
+	if (fs->free_sb)
+		fs->free_sb(s);
 
 	kill_super_notify(s);
 
@@ -661,16 +659,13 @@ EXPORT_SYMBOL(retire_super);
  *	@sb: superblock to kill
  *
  *	generic_shutdown_super() does all fs-independent work on superblock
- *	shutdown.  Typical ->kill_sb() should pick all fs-specific objects
- *	that need destruction out of superblock, call generic_shutdown_super()
- *	and release aforementioned objects.  Note: dentries and inodes _are_
- *	taken care of and do not need specific handling.
+ *	shutdown. 
  *
  *	Upon calling this function, the filesystem may no longer alter or
  *	rearrange the set of dentries belonging to this super_block, nor may it
  *	change the attachments of dentries to inodes.
  */
-void generic_shutdown_super(struct super_block *sb)
+static void generic_shutdown_super(struct super_block *sb)
 {
 	const struct super_operations *sop = sb->s_op;
 
@@ -743,8 +738,6 @@ void generic_shutdown_super(struct super_block *sb)
 	}
 }
 
-EXPORT_SYMBOL(generic_shutdown_super);
-
 bool mount_capable(struct fs_context *fc)
 {
 	if (!(fc->fs_type->fs_flags & FS_USERNS_MOUNT))
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 302be5dfc1a04a..f57d3a27b488f7 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2340,7 +2340,6 @@ struct file_system_type {
 	const struct fs_parameter_spec *parameters;
 	struct dentry *(*mount) (struct file_system_type *, int,
 		       const char *, void *);
-	void (*kill_sb) (struct super_block *);
 	void (*shutdown_sb)(struct super_block *sb);
 	void (*free_sb)(struct super_block *sb);
 	struct module *owner;
@@ -2382,7 +2381,6 @@ extern struct dentry *mount_nodev(struct file_system_type *fs_type,
 	int (*fill_super)(struct super_block *, void *, int));
 extern struct dentry *mount_subtree(struct vfsmount *mnt, const char *path);
 void retire_super(struct super_block *sb);
-void generic_shutdown_super(struct super_block *sb);
 void block_free_sb(struct super_block *sb);
 void litter_shutdown_sb(struct super_block *sb);
 void deactivate_super(struct super_block *sb);
-- 
2.39.2