[PATCH 0/3] smb/client(cifs): prepare for locking changes.

[PATCH 0/3] smb/client(cifs): prepare for locking changes.

[NeilBrown]

I am working on changes to directory locking.  The medium term goal is
to lift d_alloc_parallel() locking above i_rwsem, so d_alloc_paralle()
can be run unlocked, and consequently cannot be called when ->i_rwsem is
held.

The requires:
 1/ that d_alloc_parallel() not be called while i_rwsem is held.  cifs
    does this in a couple of places.  A new interface d_alloc_trylock()
    is now available and can be used instead.

 2/ a dentry must not be d_drop()ed while an operation is ongoing, as an
    unlocked d_alloc_parallel() would then be able to create a new
    dentry with the same name.  In particular d_rehash() must be phased
    out as it won't be safe.  This particularly affects the mechanism to
    block lookups/revalidates while an unlink/rename is happening.  The
    mechanism is changed to use one similar to how NFS addresses this
    needed, though using the new LOOKUP_PRIVATE flag.

The patches depend on patches recently added to the vfs tree.  I would
prefer these land in that tree too with an Acked-by or similar from cifs
maintainers.

Thanks,
NeilBrown

 [PATCH 1/3] smb/client: don't unhash and rehash to prevent new opens.
 [PATCH 2/3] smb/client: use d_alloc_trylock in cifs_silly_fullpath()
 [PATCH 3/3] smb/client: Use d_alloc_trylock() in cifs_prime_dcache()

[PATCH 1/3] smb/client: don't unhash and rehash to prevent new opens.

[NeilBrown]

From: NeilBrown <neil@brown.name>

smb/client needs to block new opens of the target of unlink and rename
while the operation is progressing.  This stablises d_count() and allows
a determination of whether a "silly-rename" is required.

It currently unhashes the dentry which will cause lookup to block on
the parent directory i_rwsem.  Proposed changes to locking will cause
this approach to stop working as the exclusivity will be provided for
the dentry only, and only while it is hashed.

So we introduce a new machanism similar to that used by nfs.
DCACHE_PRIVATE (given the name DCACHE_BLOCKED) is set when lookups need
to be blocked.  ->d_revalidate checks for this and blocks.  This might
still allow d_count() to increment, but once it has been tested as 1,
there can be no new opens completed.

Unlike unhash which does not need to be reverted on error, and which
must not be reverted on a successful d_move, blocking of opens must
always be reverted.  So we don't block the open until after the last
early "return", and we always unblock on the final "return".

Strictly speaking it is only necessary to block opens before the value
of d_count() is tested under ->d_lock. But we block as early as possible
to discourage new opens from starting.  As open is path-based in cifs a
concurrent rename can be confused a rename of one other the paths.  Once
cifs_open() has generated the full_path it doesn't hold any locks to
prevent a rename from making that path invalid.

The important details of the interlock between __cifs_unlink (which both
unlink and rename use) and "open" are that __cifs_unlink() sets
DCACHE_BLOCKED  *before* testing d_count() which is
in a d_lock locked region,  and cifs_d_revalidate() tests the bit *after*
->d_lock is taken by e.g. __d_lookup() to increment d_count().
Thus ->d_lock provide serialization between the set and the test
even though the test isn't in a locked region.
(The fact that cifs_d_revalidate() always returned -ECHILD when
 LOOKUP_RCU is important for this sequencing to work as it ensures
 d_revalidate() is called *after* d_count() is incremented).

Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/smb/client/cifsfs.h |  8 +++++
 fs/smb/client/dir.c    |  3 ++
 fs/smb/client/inode.c  | 68 ++++++++++++++++++++++++------------------
 3 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/fs/smb/client/cifsfs.h b/fs/smb/client/cifsfs.h
index 0c85daa8386e..bcc0fb5c2c9b 100644
--- a/fs/smb/client/cifsfs.h
+++ b/fs/smb/client/cifsfs.h
@@ -42,6 +42,14 @@ static inline unsigned long cifs_get_time(struct dentry *dentry)
 	return (unsigned long) dentry->d_fsdata;
 }
 
+/*
+ * This is set to block d_revalidate on a dentry that is being removed -
+ * the target of unlink or rename.  This causes any open attempt to
+ * block.  There may be existing opens but they can be detected by
+ * checking d_count() under ->d_lock.
+ */
+#define DCACHE_BLOCKED DCACHE_PRIVATE
+
 extern struct file_system_type cifs_fs_type, smb3_fs_type;
 extern const struct address_space_operations cifs_addr_ops;
 extern const struct address_space_operations cifs_addr_ops_smallbuf;
diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index 6fa6d48fdfd3..88fff65320f5 100644
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -872,6 +872,9 @@ cifs_d_revalidate(struct inode *dir, const struct qstr *name,
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	/* Wait for pending rename/unlink */
+	wait_var_event(&direntry->d_flags, !(direntry->d_flags & DCACHE_BLOCKED));
+
 	if (d_really_is_positive(direntry)) {
 		int rc;
 		struct inode *inode = d_inode(direntry);
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 12ed8db10e00..4d34c5622cd4 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1967,24 +1967,31 @@ static int __cifs_unlink(struct inode *dir, struct dentry *dentry, bool sillyren
 	__u32 dosattr = 0, origattr = 0;
 	struct TCP_Server_Info *server;
 	struct iattr *attrs = NULL;
-	bool rehash = false;
+	bool unblock = false;
 
 	cifs_dbg(FYI, "cifs_unlink, dir=0x%p, dentry=0x%p\n", dir, dentry);
 
 	if (unlikely(cifs_forced_shutdown(cifs_sb)))
 		return smb_EIO(smb_eio_trace_forced_shutdown);
 
-	/* Unhash dentry in advance to prevent any concurrent opens */
-	spin_lock(&dentry->d_lock);
-	if (!d_unhashed(dentry)) {
-		__d_drop(dentry);
-		rehash = true;
-	}
-	spin_unlock(&dentry->d_lock);
-
 	tlink = cifs_sb_tlink(cifs_sb);
 	if (IS_ERR(tlink))
 		return PTR_ERR(tlink);
+
+	/* opens might already be blocked by rename */
+	if (!(dentry->d_flags & DCACHE_BLOCKED)) {
+		/*
+		 * Block opens - and all lookups that involve d_revalidate.
+		 * This guarantees that if another thread tries to open(), it
+		 * will either block, or will increment d_count()
+		 * before we test it below.
+		 */
+		spin_lock(&dentry->d_lock);
+		dentry->d_flags |= DCACHE_BLOCKED;
+		spin_unlock(&dentry->d_lock);
+		unblock = true;
+	}
+
 	tcon = tlink_tcon(tlink);
 	server = tcon->ses->server;
 
@@ -2107,8 +2114,13 @@ static int __cifs_unlink(struct inode *dir, struct dentry *dentry, bool sillyren
 	kfree(attrs);
 	free_xid(xid);
 	cifs_put_tlink(tlink);
-	if (rehash)
-		d_rehash(dentry);
+	/* Allow lookups/opens */
+	if (unblock) {
+		spin_lock(&dentry->d_lock);
+		store_release_wake_up(&dentry->d_flags,
+				      dentry->d_flags &~ DCACHE_BLOCKED);
+		spin_unlock(&dentry->d_lock);
+	}
 	return rc;
 }
 
@@ -2536,7 +2548,6 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
 	struct cifs_sb_info *cifs_sb;
 	struct tcon_link *tlink;
 	struct cifs_tcon *tcon;
-	bool rehash = false;
 	unsigned int xid;
 	int rc, tmprc;
 	int retry_count = 0;
@@ -2552,20 +2563,20 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
 	if (unlikely(cifs_forced_shutdown(cifs_sb)))
 		return smb_EIO(smb_eio_trace_forced_shutdown);
 
-	/*
-	 * Prevent any concurrent opens on the target by unhashing the dentry.
-	 * VFS already unhashes the target when renaming directories.
-	 */
-	if (d_is_positive(target_dentry) && !d_is_dir(target_dentry)) {
-		if (!d_unhashed(target_dentry)) {
-			d_drop(target_dentry);
-			rehash = true;
-		}
-	}
-
 	tlink = cifs_sb_tlink(cifs_sb);
 	if (IS_ERR(tlink))
 		return PTR_ERR(tlink);
+
+	/*
+	 * Block opens - and all lookups that involve d_revalidate.
+	 * This guarantees that if another thread tries to open(), it
+	 * will either block, or will increment d_count()
+	 * before we test it in __cifs_unlink().
+	 */
+	spin_lock(&target_dentry->d_lock);
+	target_dentry->d_flags |= DCACHE_BLOCKED;
+	spin_unlock(&target_dentry->d_lock);
+
 	tcon = tlink_tcon(tlink);
 	server = tcon->ses->server;
 
@@ -2605,8 +2616,6 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
 		}
 	}
 
-	if (!rc)
-		rehash = false;
 	/*
 	 * No-replace is the natural behavior for CIFS, so skip unlink hacks.
 	 */
@@ -2698,8 +2707,6 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
 			}
 			rc = cifs_do_rename(xid, source_dentry, from_name,
 					    target_dentry, to_name);
-			if (!rc)
-				rehash = false;
 		}
 	}
 
@@ -2713,8 +2720,11 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
 	CIFS_I(source_dir)->time = CIFS_I(target_dir)->time = 0;
 
 cifs_rename_exit:
-	if (rehash)
-		d_rehash(target_dentry);
+	/* Allow lookups/opens */
+	spin_lock(&target_dentry->d_lock);
+	store_release_wake_up(&target_dentry->d_flags,
+			      target_dentry->d_flags &~ DCACHE_BLOCKED);
+	spin_unlock(&target_dentry->d_lock);
 	kfree(info_buf_source);
 	free_dentry_path(page2);
 	free_dentry_path(page1);

base-commit: 3879f51857325da9bf3cfb073280257cd16ae067
-- 
2.50.0.107.gf914562f5916.dirty

Re: [PATCH 1/3] smb/client: don't unhash and rehash to prevent new opens.

[Tom Talpey]

One nit below...

On 9/27/2026 9:25 PM, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
> 
> smb/client needs to block new opens of the target of unlink and rename
> while the operation is progressing.  This stablises d_count() and allows
> a determination of whether a "silly-rename" is required.
> 
> It currently unhashes the dentry which will cause lookup to block on
> the parent directory i_rwsem.  Proposed changes to locking will cause
> this approach to stop working as the exclusivity will be provided for
> the dentry only, and only while it is hashed.
> 
> So we introduce a new machanism similar to that used by nfs.
> DCACHE_PRIVATE (given the name DCACHE_BLOCKED) is set when lookups need
> to be blocked.  ->d_revalidate checks for this and blocks.  This might
> still allow d_count() to increment, but once it has been tested as 1,
> there can be no new opens completed.
> 
> Unlike unhash which does not need to be reverted on error, and which
> must not be reverted on a successful d_move, blocking of opens must
> always be reverted.  So we don't block the open until after the last
> early "return", and we always unblock on the final "return".
> 
> Strictly speaking it is only necessary to block opens before the value
> of d_count() is tested under ->d_lock. But we block as early as possible
> to discourage new opens from starting.  As open is path-based in cifs a
> concurrent rename can be confused a rename of one other the paths.  Once
> cifs_open() has generated the full_path it doesn't hold any locks to
> prevent a rename from making that path invalid.
> 
> The important details of the interlock between __cifs_unlink (which both
> unlink and rename use) and "open" are that __cifs_unlink() sets
> DCACHE_BLOCKED  *before* testing d_count() which is
> in a d_lock locked region,  and cifs_d_revalidate() tests the bit *after*
> ->d_lock is taken by e.g. __d_lookup() to increment d_count().
> Thus ->d_lock provide serialization between the set and the test
> even though the test isn't in a locked region.
> (The fact that cifs_d_revalidate() always returned -ECHILD when
>   LOOKUP_RCU is important for this sequencing to work as it ensures
>   d_revalidate() is called *after* d_count() is incremented).
> 
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>   fs/smb/client/cifsfs.h |  8 +++++
>   fs/smb/client/dir.c    |  3 ++
>   fs/smb/client/inode.c  | 68 ++++++++++++++++++++++++------------------
>   3 files changed, 50 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/smb/client/cifsfs.h b/fs/smb/client/cifsfs.h
> index 0c85daa8386e..bcc0fb5c2c9b 100644
> --- a/fs/smb/client/cifsfs.h
> +++ b/fs/smb/client/cifsfs.h
> @@ -42,6 +42,14 @@ static inline unsigned long cifs_get_time(struct dentry *dentry)
>   	return (unsigned long) dentry->d_fsdata;
>   }
>   
> +/*
> + * This is set to block d_revalidate on a dentry that is being removed -
> + * the target of unlink or rename.  This causes any open attempt to
> + * block.  There may be existing opens but they can be detected by
> + * checking d_count() under ->d_lock.
> + */
> +#define DCACHE_BLOCKED DCACHE_PRIVATE

Why the alias? The blocking behavior is reasonable but #defining a
flag value which is purely local to this file seems unnecessary and
confusing. What if another file in fs/smb/client also overloads the
DCACHE_PRIVATE flag, for example. Consider removing this hunk.

Tom.


> +
>   extern struct file_system_type cifs_fs_type, smb3_fs_type;
>   extern const struct address_space_operations cifs_addr_ops;
>   extern const struct address_space_operations cifs_addr_ops_smallbuf;
> diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
> index 6fa6d48fdfd3..88fff65320f5 100644
> --- a/fs/smb/client/dir.c
> +++ b/fs/smb/client/dir.c
> @@ -872,6 +872,9 @@ cifs_d_revalidate(struct inode *dir, const struct qstr *name,
>   	if (flags & LOOKUP_RCU)
>   		return -ECHILD;
>   
> +	/* Wait for pending rename/unlink */
> +	wait_var_event(&direntry->d_flags, !(direntry->d_flags & DCACHE_BLOCKED));
> +
>   	if (d_really_is_positive(direntry)) {
>   		int rc;
>   		struct inode *inode = d_inode(direntry);
> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> index 12ed8db10e00..4d34c5622cd4 100644
> --- a/fs/smb/client/inode.c
> +++ b/fs/smb/client/inode.c
> @@ -1967,24 +1967,31 @@ static int __cifs_unlink(struct inode *dir, struct dentry *dentry, bool sillyren
>   	__u32 dosattr = 0, origattr = 0;
>   	struct TCP_Server_Info *server;
>   	struct iattr *attrs = NULL;
> -	bool rehash = false;
> +	bool unblock = false;
>   
>   	cifs_dbg(FYI, "cifs_unlink, dir=0x%p, dentry=0x%p\n", dir, dentry);
>   
>   	if (unlikely(cifs_forced_shutdown(cifs_sb)))
>   		return smb_EIO(smb_eio_trace_forced_shutdown);
>   
> -	/* Unhash dentry in advance to prevent any concurrent opens */
> -	spin_lock(&dentry->d_lock);
> -	if (!d_unhashed(dentry)) {
> -		__d_drop(dentry);
> -		rehash = true;
> -	}
> -	spin_unlock(&dentry->d_lock);
> -
>   	tlink = cifs_sb_tlink(cifs_sb);
>   	if (IS_ERR(tlink))
>   		return PTR_ERR(tlink);
> +
> +	/* opens might already be blocked by rename */
> +	if (!(dentry->d_flags & DCACHE_BLOCKED)) {
> +		/*
> +		 * Block opens - and all lookups that involve d_revalidate.
> +		 * This guarantees that if another thread tries to open(), it
> +		 * will either block, or will increment d_count()
> +		 * before we test it below.
> +		 */
> +		spin_lock(&dentry->d_lock);
> +		dentry->d_flags |= DCACHE_BLOCKED;
> +		spin_unlock(&dentry->d_lock);
> +		unblock = true;
> +	}
> +
>   	tcon = tlink_tcon(tlink);
>   	server = tcon->ses->server;
>   
> @@ -2107,8 +2114,13 @@ static int __cifs_unlink(struct inode *dir, struct dentry *dentry, bool sillyren
>   	kfree(attrs);
>   	free_xid(xid);
>   	cifs_put_tlink(tlink);
> -	if (rehash)
> -		d_rehash(dentry);
> +	/* Allow lookups/opens */
> +	if (unblock) {
> +		spin_lock(&dentry->d_lock);
> +		store_release_wake_up(&dentry->d_flags,
> +				      dentry->d_flags &~ DCACHE_BLOCKED);
> +		spin_unlock(&dentry->d_lock);
> +	}
>   	return rc;
>   }
>   
> @@ -2536,7 +2548,6 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
>   	struct cifs_sb_info *cifs_sb;
>   	struct tcon_link *tlink;
>   	struct cifs_tcon *tcon;
> -	bool rehash = false;
>   	unsigned int xid;
>   	int rc, tmprc;
>   	int retry_count = 0;
> @@ -2552,20 +2563,20 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
>   	if (unlikely(cifs_forced_shutdown(cifs_sb)))
>   		return smb_EIO(smb_eio_trace_forced_shutdown);
>   
> -	/*
> -	 * Prevent any concurrent opens on the target by unhashing the dentry.
> -	 * VFS already unhashes the target when renaming directories.
> -	 */
> -	if (d_is_positive(target_dentry) && !d_is_dir(target_dentry)) {
> -		if (!d_unhashed(target_dentry)) {
> -			d_drop(target_dentry);
> -			rehash = true;
> -		}
> -	}
> -
>   	tlink = cifs_sb_tlink(cifs_sb);
>   	if (IS_ERR(tlink))
>   		return PTR_ERR(tlink);
> +
> +	/*
> +	 * Block opens - and all lookups that involve d_revalidate.
> +	 * This guarantees that if another thread tries to open(), it
> +	 * will either block, or will increment d_count()
> +	 * before we test it in __cifs_unlink().
> +	 */
> +	spin_lock(&target_dentry->d_lock);
> +	target_dentry->d_flags |= DCACHE_BLOCKED;
> +	spin_unlock(&target_dentry->d_lock);
> +
>   	tcon = tlink_tcon(tlink);
>   	server = tcon->ses->server;
>   
> @@ -2605,8 +2616,6 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
>   		}
>   	}
>   
> -	if (!rc)
> -		rehash = false;
>   	/*
>   	 * No-replace is the natural behavior for CIFS, so skip unlink hacks.
>   	 */
> @@ -2698,8 +2707,6 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
>   			}
>   			rc = cifs_do_rename(xid, source_dentry, from_name,
>   					    target_dentry, to_name);
> -			if (!rc)
> -				rehash = false;
>   		}
>   	}
>   
> @@ -2713,8 +2720,11 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
>   	CIFS_I(source_dir)->time = CIFS_I(target_dir)->time = 0;
>   
>   cifs_rename_exit:
> -	if (rehash)
> -		d_rehash(target_dentry);
> +	/* Allow lookups/opens */
> +	spin_lock(&target_dentry->d_lock);
> +	store_release_wake_up(&target_dentry->d_flags,
> +			      target_dentry->d_flags &~ DCACHE_BLOCKED);
> +	spin_unlock(&target_dentry->d_lock);
>   	kfree(info_buf_source);
>   	free_dentry_path(page2);
>   	free_dentry_path(page1);
> 
> base-commit: 3879f51857325da9bf3cfb073280257cd16ae067

[PATCH 2/3] smb/client: use d_alloc_trylock in cifs_silly_fullpath()

[NeilBrown]

From: NeilBrown <neil@brown.name>

Rather than performing a normal lookup (which will be awkward with future
locking changes) use d_alloc_trylock() to find a dentry for an
unused name, and use an open-coded lookup_slow() to see if it is free on
the server.

Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/smb/client/dir.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index 88fff65320f5..2faac93c6758 100644
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -1183,23 +1183,34 @@ char *cifs_silly_fullpath(struct dentry *dentry)
 	unsigned char name[CIFS_SILLYNAME_LEN + 1];
 	int retries = 0, max_retries = 16;
 	size_t namesize = sizeof(name);
-	struct dentry *sdentry = NULL;
+	struct dentry *sdentry = NULL, *alias;
 	char *path;
 
 	do {
 		dput(sdentry);
 		scnprintf(name, namesize, CIFS_SILLYNAME_PREFIX "%x",
 			  atomic_inc_return(&cifs_sillycounter));
-		sdentry = lookup_noperm(&QSTR(name), dentry->d_parent);
+		sdentry = d_alloc_trylock(dentry->d_parent, &QSTR(name));
+		if (sdentry == ERR_PTR(-EWOULDBLOCK)) {
+			sdentry = NULL;
+			continue;
+		}
 		if (IS_ERR(sdentry))
 			return ERR_CAST(sdentry);
-		if (d_is_negative(sdentry)) {
-			dput(sdentry);
-			path = alloc_parent_path(dentry, CIFS_SILLYNAME_LEN);
-			if (!IS_ERR(path))
-				strcat(path, name);
-			return path;
+		if (!d_in_lookup(sdentry))
+			continue;
+		alias = cifs_lookup(sdentry->d_parent->d_inode, sdentry, 0);
+		d_lookup_done(sdentry);
+		if (alias || d_is_positive(dentry)) {
+			if (!IS_ERR(alias))
+				dput(alias);
+			continue;
 		}
+		dput(sdentry);
+		path = alloc_parent_path(dentry, CIFS_SILLYNAME_LEN);
+		if (!IS_ERR(path))
+			strcat(path, name);
+		return path;
 	} while (++retries < max_retries);
 	dput(sdentry);
 	return ERR_PTR(-EBUSY);
-- 
2.50.0.107.gf914562f5916.dirty

[PATCH 3/3] smb/client: Use d_alloc_trylock() in cifs_prime_dcache()

[NeilBrown]

From: NeilBrown <neil@brown.name>

cifs uses the results of readdir to prime the dcache.  Using
d_alloc_parallel() can block if there is a concurrent lookup.  Blocking
in that case is pointless as the lookup will add info to the dcache and
there is no value in the readdir waiting to see if it should add the
info too.

Also this call to d_alloc_parallel() is made while the parent
directory is locked.  A proposed change to locking will lock the parent
later, after d_alloc_parallel().  This means it won't be safe to wait in
d_alloc_parallel() while holding the directory lock.

So change to use d_alloc_trylock().  As d_alloc_trylock() includes a
try_lookup we don't need both.   Rearrange the code so we can choose
either a try_lookup or d_alloc_trylock.

Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/smb/client/readdir.c | 50 ++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 32a75afca8f5..19d0c77046ab 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -77,36 +77,34 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
 
 	cifs_dbg(FYI, "%s: for %s\n", __func__, name->name);
 
-	dentry = try_lookup_noperm(name, parent);
-	if (!dentry) {
-		/*
-		 * If we know that the inode will need to be revalidated
-		 * immediately, then don't create a new dentry for it.
-		 * We'll end up doing an on the wire call either way and
-		 * this spares us an invalidation.
-		 */
-retry:
-		if (posix) {
-			switch (fattr->cf_mode & S_IFMT) {
-			case S_IFLNK:
-			case S_IFBLK:
-			case S_IFCHR:
-				reparse_need_reval = true;
-				break;
-			default:
-				break;
-			}
-		} else if (fattr->cf_cifsattrs & ATTR_REPARSE_POINT) {
+	/*
+	 * If we know that the inode will need to be revalidated
+	 * immediately, then don't create a new dentry for it.
+	 * We'll end up doing an on the wire call either way and
+	 * this spares us an invalidation.
+	 */
+	if (posix) {
+		switch (fattr->cf_mode & S_IFMT) {
+		case S_IFLNK:
+		case S_IFBLK:
+		case S_IFCHR:
 			reparse_need_reval = true;
+			break;
+		default:
+			break;
 		}
+	} else if (fattr->cf_cifsattrs & ATTR_REPARSE_POINT) {
+		reparse_need_reval = true;
+	}
 
-		if (reparse_need_reval ||
-		    (fattr->cf_flags & CIFS_FATTR_NEED_REVAL))
-			return;
+retry:
+	if (reparse_need_reval ||
+	    (fattr->cf_flags & CIFS_FATTR_NEED_REVAL))
+		dentry = try_lookup_noperm(name, parent);
+	else
+		dentry = d_alloc_trylock(parent, name);
 
-		dentry = d_alloc_parallel(parent, name);
-	}
-	if (IS_ERR(dentry))
+	if (!dentry || IS_ERR(dentry))
 		return;
 	if (!d_in_lookup(dentry)) {
 		inode = d_inode(dentry);
-- 
2.50.0.107.gf914562f5916.dirty

Re: [PATCH 0/3] smb/client(cifs): prepare for locking changes.

[Christian Brauner]

On Mon, Sep 28, 2026 at 11:25:01AM +1000, NeilBrown wrote:
> I am working on changes to directory locking.  The medium term goal is
> to lift d_alloc_parallel() locking above i_rwsem, so d_alloc_paralle()
> can be run unlocked, and consequently cannot be called when ->i_rwsem is
> held.
> 
> The requires:
>  1/ that d_alloc_parallel() not be called while i_rwsem is held.  cifs
>     does this in a couple of places.  A new interface d_alloc_trylock()
>     is now available and can be used instead.
> 
>  2/ a dentry must not be d_drop()ed while an operation is ongoing, as an
>     unlocked d_alloc_parallel() would then be able to create a new
>     dentry with the same name.  In particular d_rehash() must be phased
>     out as it won't be safe.  This particularly affects the mechanism to
>     block lookups/revalidates while an unlink/rename is happening.  The
>     mechanism is changed to use one similar to how NFS addresses this
>     needed, though using the new LOOKUP_PRIVATE flag.
> 
> The patches depend on patches recently added to the vfs tree.  I would
> prefer these land in that tree too with an Acked-by or similar from cifs
> maintainers.

Can do, will need an Ack.

Re: [PATCH 0/3] smb/client(cifs): prepare for locking changes.

[Tom Talpey]

On 10/21/2026 5:55 AM, Christian Brauner wrote:
> On Mon, Sep 28, 2026 at 11:25:01AM +1000, NeilBrown wrote:
>> I am working on changes to directory locking.  The medium term goal is
>> to lift d_alloc_parallel() locking above i_rwsem, so d_alloc_paralle()
>> can be run unlocked, and consequently cannot be called when ->i_rwsem is
>> held.
>>
>> The requires:
>>   1/ that d_alloc_parallel() not be called while i_rwsem is held.  cifs
>>      does this in a couple of places.  A new interface d_alloc_trylock()
>>      is now available and can be used instead.
>>
>>   2/ a dentry must not be d_drop()ed while an operation is ongoing, as an
>>      unlocked d_alloc_parallel() would then be able to create a new
>>      dentry with the same name.  In particular d_rehash() must be phased
>>      out as it won't be safe.  This particularly affects the mechanism to
>>      block lookups/revalidates while an unlink/rename is happening.  The
>>      mechanism is changed to use one similar to how NFS addresses this
>>      needed, though using the new LOOKUP_PRIVATE flag.
>>
>> The patches depend on patches recently added to the vfs tree.  I would
>> prefer these land in that tree too with an Acked-by or similar from cifs
>> maintainers.
> 
> Can do, will need an Ack.
I will defer to Paulo for the Ack, but for the series, modulo my nit
on DCACHE_BLOCKED:

Reviewed-by: Tom Talpey <tom@talpey.com>