linux-kernel - Re: [PATCH RFC 00/12] Allow concurrent directory updates.

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-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 17 Jun 2022 15:49:41 +1000
From:   "NeilBrown" <neilb@...e.de>
To:     "Daire Byrne" <daire@...g.com>,
        Anna Schumaker <schumaker.anna@...il.com>
Cc:     "Al Viro" <viro@...iv.linux.org.uk>,
        "Trond Myklebust" <trond.myklebust@...merspace.com>,
        "Chuck Lever" <chuck.lever@...cle.com>,
        "Linux NFS Mailing List" <linux-nfs@...r.kernel.org>,
        linux-fsdevel@...r.kernel.org,
        "LKML" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC 00/12] Allow concurrent directory updates.

On Thu, 16 Jun 2022, Daire Byrne wrote:
> 
> I double checked that the patch had been applied and I hadn't made a
> mistake with installation.

:-) always worth double checking...

> 
> I could perhaps try running with just the VFS patches to see if I can
> still reproduce the "local" VFS hang without the nfsd patches? Your
> previous VFS only patchset was stable for me.

I've made quite a few changes since that VFS-only patches.  Almost
certainly the problem is not in the nfsd code.

I think that following has a reasonable chance of making things better,
both for the problem you hit and the problem Anna hit.  I haven't tested
it at all yet so no promises - up to you if you try it.

Thanks to both of you for the help with testing.

NeilBrown
 

diff --git a/fs/namei.c b/fs/namei.c
index 31ba4dbedfcf..6d0c955d407a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1609,7 +1609,7 @@ static struct dentry *__lookup_hash(const struct qstr *name,
 	if (IS_ERR(dentry))
 		return dentry;
 
-	if (wq && d_in_lookup(dentry))
+	if (wq && !d_in_lookup(dentry))
 		/* Must have raced with another thread doing the lookup */
 		return dentry;
 
@@ -1664,6 +1664,7 @@ static struct dentry *lookup_hash_update(const struct qstr *name,
 	}
 	if (flags & LOOKUP_EXCL) {
 		if (d_is_positive(dentry)) {
+			d_lookup_done(dentry);
 			dput(dentry);
 			err = -EEXIST;
 			goto out_err;
@@ -1671,6 +1672,7 @@ static struct dentry *lookup_hash_update(const struct qstr *name,
 	}
 	if (!(flags & LOOKUP_CREATE)) {
 		if (!dentry->d_inode) {
+			d_lookup_done(dentry);
 			dput(dentry);
 			err = -ENOENT;
 			goto out_err;
@@ -1687,6 +1689,8 @@ static struct dentry *lookup_hash_update(const struct qstr *name,
 	}
 	if (err2) {
 		err = err2;
+		d_lookup_done(dentry);
+		d_unlock_update(dentry);
 		dput(dentry);
 		goto out_err;
 	}
@@ -3273,6 +3277,7 @@ static struct dentry *lock_rename_lookup(struct dentry *p1, struct dentry *p2,
 		}
 		return NULL;
 	out_unlock_2:
+		d_lookup_done(d1);
 		dput(d1);
 		d1 = d2;
 	out_unlock_1:
@@ -3315,6 +3320,7 @@ static struct dentry *lock_rename_lookup(struct dentry *p1, struct dentry *p2,
 	*d2p = d2;
 	return p;
 unlock_out_4:
+	d_lookup_done(d1);
 	dput(d1);
 	d1 = d2;
 unlock_out_3: