linux-kernel - [PATCH] fs/super.c: defer more work after super_block is off of the super

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 22 Oct 2021 16:28:46 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     viro@...iv.linux.org.uk
Cc:     neilb@...e.de, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, Luis Chamberlain <mcgrof@...nel.org>
Subject: [PATCH] fs/super.c: defer more work after super_block is off of the super_blocks list

Once it is off the super_blocks list we don't really need to hold the
sb_lock anymore, we can defer the rest of the work. This reduces a few
uneeded operations from contention from the sb_lock.

This is a minor optimization found through code inspection. If the
sb_lock is not needed, no need for contention to wait while we free
items. While at it, add a bit of documentation about the extent to
which the sb_lock is used.

Signed-off-by: Luis Chamberlain <mcgrof@...nel.org>
---
 fs/super.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index bcef3a6f4c4b..24a76490c46f 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -43,6 +43,13 @@
 static int thaw_super_locked(struct super_block *sb);
 
 static LIST_HEAD(super_blocks);
+
+/**
+ * Protects the super_blocks list as well as the respective fs_type->fs_supers.
+ * When removing a super block it is first removed from the fs_type->fs_supers
+ * through generic_shutdown_super(). The final nail on the super block is the
+ * last temporary reference with __put_super().
+ */
 static DEFINE_SPINLOCK(sb_lock);
 
 static char *sb_writers_name[SB_FREEZE_LEVELS] = {
@@ -160,6 +167,11 @@ static void destroy_super_work(struct work_struct *work)
 							destroy_work);
 	int i;
 
+	security_sb_free(s);
+	fscrypt_sb_free(s);
+	put_user_ns(s->s_user_ns);
+	kfree(s->s_subtype);
+
 	for (i = 0; i < SB_FREEZE_LEVELS; i++)
 		percpu_free_rwsem(&s->s_writers.rw_sem[i]);
 	kfree(s);
@@ -292,10 +304,6 @@ static void __put_super(struct super_block *s)
 		WARN_ON(s->s_dentry_lru.node);
 		WARN_ON(s->s_inode_lru.node);
 		WARN_ON(!list_empty(&s->s_mounts));
-		security_sb_free(s);
-		fscrypt_sb_free(s);
-		put_user_ns(s->s_user_ns);
-		kfree(s->s_subtype);
 		call_rcu(&s->rcu, destroy_super_rcu);
 	}
 }
@@ -471,7 +479,7 @@ void generic_shutdown_super(struct super_block *sb)
 		}
 	}
 	spin_lock(&sb_lock);
-	/* should be initialized for __put_super_and_need_restart() */
+	/* and so __put_super() does not need to deal with this */
 	hlist_del_init(&sb->s_instances);
 	spin_unlock(&sb_lock);
 	up_write(&sb->s_umount);
-- 
2.30.2