[15/19] kernfs: split ->kill_sb

Commit Message

Christoph Hellwig Sept. 13, 2023, 11:10 a.m. UTC

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 <[e-mail]>
---
 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(-)

Comments

Michal Koutný Sept. 18, 2023, 3:24 p.m. UTC | #1

On Wed, Sep 13, 2023 at 08:10:09AM -0300, Christoph Hellwig <[e-mail]> 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 <[e-mail]>
> ---
>  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ý <[e-mail]>

(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.)

Patch

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,
 };