linux-kernel - [PATCH/RFC] VFS: Improve fairness when locking the per-superblock s

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, 29 Jan 2016 11:17:43 +1100
From:	NeilBrown <neilb@...e.com>
To:	Al Viro <viro@...IV.linux.org.uk>,
	Dave Chinner <dchinner@...hat.com>
Cc:	LKML <linux-kernel@...r.kernel.org>, linux-nfs@...r.kernel.org
Subject: [PATCH/RFC] VFS: Improve fairness when locking the per-superblock s_anon list

bit-spin-locks, as used for dcache hash chains, are not fair.
This is not a problem for the dcache hash table as different CPUs are
likely to access different entries in the hash table so high contention
is not expected.
However anonymous dentryies (created by NFSD) all live on a single hash
chain "s_anon" and the bitlock on this can be highly contended, resulting
in soft-lockup warnings.

So introduce a global (fair) spinlock and take it before grabing the
bitlock on s_anon.  This provides fairness and makes the warnings go away.

We could alternately use s_inode_list_lock, or add another spinlock
to struct super_block.  Suggestions?

Signed-off-by: NeilBrown <neilb@...e.com>
---

Dave: I'd guess you would be against using the new s_inode_list_lock
for this because it is already highly contended - yes?
Is it worth adding another spinlock to 'struct super_block' ?

Thanks,
NeilBrown

 fs/dcache.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/dcache.c b/fs/dcache.c
index 92d5140de851..e83f1ac1540c 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -104,6 +104,8 @@ static unsigned int d_hash_shift __read_mostly;

 static struct hlist_bl_head *dentry_hashtable __read_mostly;

+static DEFINE_SPINLOCK(s_anon_lock);
+
 static inline struct hlist_bl_head *d_hash(const struct dentry *parent,
 					unsigned int hash)
 {
@@ -490,10 +492,14 @@ void __d_drop(struct dentry *dentry)
 		else
 			b = d_hash(dentry->d_parent, dentry->d_name.hash);

+		if (b == &dentry->d_sb->s_anon)
+			spin_lock(&s_anon_lock);
 		hlist_bl_lock(b);
 		__hlist_bl_del(&dentry->d_hash);
 		dentry->d_hash.pprev = NULL;
 		hlist_bl_unlock(b);
+		if (b == &dentry->d_sb->s_anon)
+			spin_unlock(&s_anon_lock);
 		dentry_rcuwalk_invalidate(dentry);
 	}
 }
@@ -1978,9 +1984,11 @@ static struct dentry *__d_obtain_alias(struct inode *inode, int disconnected)
 	spin_lock(&tmp->d_lock);
 	__d_set_inode_and_type(tmp, inode, add_flags);
 	hlist_add_head(&tmp->d_u.d_alias, &inode->i_dentry);
+	spin_lock(&s_anon_lock);
 	hlist_bl_lock(&tmp->d_sb->s_anon);
 	hlist_bl_add_head(&tmp->d_hash, &tmp->d_sb->s_anon);
 	hlist_bl_unlock(&tmp->d_sb->s_anon);
+	spin_unlock(&s_anon_lock);
 	spin_unlock(&tmp->d_lock);
 	spin_unlock(&inode->i_lock);
 	security_d_instantiate(tmp, inode);
-- 
2.7.0

Download attachment "signature.asc" of type "application/pgp-signature" (819 bytes)