* [PATCH v3 1/9] nfs: fix open-blocking with d_fsdata
2026-10-23 1:50 [PATCH v3 0/9] nfs: prepare for locking changes NeilBrown
@ 2026-10-23 1:50 ` NeilBrown
2026-10-23 1:50 ` [PATCH v3 2/9] nfs: fix nfs_call_unlink() NeilBrown
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: NeilBrown @ 2026-10-23 1:50 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, Alexander Viro, Christian Brauner
Cc: Jeff Layton, Jan Kara, linux-fsdevel, linux-nfs, linux-kernel
From: NeilBrown <neil@brown.name>
The logic described in block_revalidate() that ->d_lock must be taken in
lookup_open() missed the fact that sometimes lookup_open() isn't called.
If open_last_lookups() is called while still in RCU-walk and
lookup_fast() finds a dentry, it does not take ->d_lock or increment
d_count() before calling d_revalidate() - only afterwards.
So on that path d_count() can be incremented and the file opened *after*
nfs_unlink() has checked d_count() and set NFS_FSDATA_BLOCKED. So an
application could have the file open when it is unlinked on the server.
To fix this we need block_revalidate() to ensure the RCU-walk path
is blocked as well. This is easily done with
write_seqcount_invalidate(&dentry->d_seq);
legitimize_path() will increment d_count() and then check d_seq.
The write_seqcount_invalidate() call with cause that check to fail if
nfs_unlink() has already seen d_count() being 1. So the RCU-walk will
fail with -ECHILD and it will be retried as a ref-walk.
Also the smp_load_acquire() in __nfs_lookup_revalidate() is pointless.
wait_var_event() contains the required barriers (a spinlock in
prepare_to_wait_event()) to ensure the correct value is read after any
wakeup.
That barrier doesn't apply for the very first test of the condition in
wait_var_event) but the locking of ->d_lock described above provides
sufficient barriers.
Fixes: 99bc9f2eb3f7 ("NFS: add barriers when testing for NFS_FSDATA_BLOCKED")
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/nfs/dir.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 354f986e60c4..d26b09839d88 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1837,8 +1837,7 @@ __nfs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
} else {
/* Wait for unlink to complete - see unblock_revalidate() */
wait_var_event(&dentry->d_fsdata,
- smp_load_acquire(&dentry->d_fsdata)
- != NFS_FSDATA_BLOCKED);
+ dentry->d_fsdata != NFS_FSDATA_BLOCKED);
}
return 0;
}
@@ -1857,12 +1856,15 @@ static void block_revalidate(struct dentry *dentry)
kfree(dentry->d_fsdata);
/* Any new reference that could lead to an open
- * will take ->d_lock in lookup_open() -> d_lookup().
- * Holding this lock ensures we cannot race with
- * __nfs_lookup_revalidate() and removes and need
- * for further barriers.
+ * will either:
+ * - take ->d_lock in lookup_open() -> d_lookup() or
+ * - will check d_seq in legitimize_mnt()
+ *
+ * Holding this lock and invalidating ->d_seq ensures we cannot
+ * race with __nfs_lookup_revalidate().
*/
lockdep_assert_held(&dentry->d_lock);
+ write_seqcount_invalidate(&dentry->d_seq);
dentry->d_fsdata = NFS_FSDATA_BLOCKED;
}
base-commit: 864c07de0c7dcbca58b61625dfa2e461ae63b242
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 2/9] nfs: fix nfs_call_unlink()
2026-10-23 1:50 [PATCH v3 0/9] nfs: prepare for locking changes NeilBrown
2026-10-23 1:50 ` [PATCH v3 1/9] nfs: fix open-blocking with d_fsdata NeilBrown
@ 2026-10-23 1:50 ` NeilBrown
2026-10-23 1:50 ` [PATCH v3 3/9] nfs: remove d_drop()/d_alloc_parallel() from nfs_atomic_open() NeilBrown
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: NeilBrown @ 2026-10-23 1:50 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, Alexander Viro, Christian Brauner
Cc: Jeff Layton, Jan Kara, linux-fsdevel, linux-nfs, linux-kernel
From: NeilBrown <neil@brown.name>
nfs_call_unlink() calls d_alloc_parallel() on a dentry which has not had
the d_hash calculated. d_alloc_parallel() assume this is calculated and
doesn't calculate it itself.
So this will never find an existing dentry and so will not detect
the races it aims to detect.
This dentry ("alias") is never hashed so there is no lasting
inconsistency in the dcache.
Fixes: 565277f63c61 ("NFS: Fix a race in sillyrename")
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/nfs/unlink.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index c8d712204e64..11a46a993e11 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -125,6 +125,9 @@ static int nfs_call_unlink(struct dentry *dentry, struct inode *inode, struct nf
struct dentry *alias;
down_read_non_owner(&NFS_I(dir)->rmdir_sem);
+ data->args.name.hash = full_name_hash(dentry->d_parent,
+ data->args.name.name,
+ data->args.name.len);
alias = d_alloc_parallel(dentry->d_parent, &data->args.name);
if (IS_ERR(alias)) {
up_read_non_owner(&NFS_I(dir)->rmdir_sem);
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 3/9] nfs: remove d_drop()/d_alloc_parallel() from nfs_atomic_open()
2026-10-23 1:50 [PATCH v3 0/9] nfs: prepare for locking changes NeilBrown
2026-10-23 1:50 ` [PATCH v3 1/9] nfs: fix open-blocking with d_fsdata NeilBrown
2026-10-23 1:50 ` [PATCH v3 2/9] nfs: fix nfs_call_unlink() NeilBrown
@ 2026-10-23 1:50 ` NeilBrown
2026-10-23 1:50 ` [PATCH v3 4/9] nfs: use d_splice_alias() in nfs_link() NeilBrown
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: NeilBrown @ 2026-10-23 1:50 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, Alexander Viro, Christian Brauner
Cc: Jeff Layton, Jan Kara, linux-fsdevel, linux-nfs, linux-kernel
From: NeilBrown <neil@brown.name>
It is important that two non-create NFS "open"s of a negative dentry
don't race. They both have only a shared lock on i_rwsem and so could
run concurrently, but they might both try to call d_splice_alias() at
the same time which is confusing at best.
nfs_atomic_open() currently avoids this by discarding the negative
dentry and creating a new one using d_alloc_parallel(). Only one thread
can successfully get the d_in_lookup() dentry, the other will wait for
the first to finish, and can use the result of that first lookup.
A proposed locking change inverts the order between i_rwsem and
d_alloc_parallel() so it will not be safe to call d_alloc_parallel()
while holding i_rwsem - even shared.
We can achieve the same effect by causing ->d_revalidate to invalidate a
negative dentry when LOOKUP_OPEN is set. Doing this is consistent with
the "close to open" caching semantics of NFS which requires the server
to be queried whenever opening a file - cached information must not be
trusted.
With this change to ->d_revaliate (implemented in nfs_neg_need_reval())
we can be sure that we have exclusive access to any dentry that reaches
nfs_atomic_open(). Either O_CREAT was requested and so the parent is
locked exclusively, or the dentry will have DCACHE_PAR_LOOKUP set.
[Note that when nfs_neg_need_reval() returns 1, ->d_revalidate returns 0].
This means that the d_drop() and d_alloc_parallel() calls in
nfs_atomic_lookup() are no longer needed to provide exclusion
There is still a d_drop() which allowed d_splice_alias() to succeed.
This will be removed in a future patch.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/nfs/dir.c | 30 +++++++-----------------------
1 file changed, 7 insertions(+), 23 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index d26b09839d88..59981a4c7403 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1656,6 +1656,13 @@ int nfs_neg_need_reval(struct inode *dir, struct dentry *dentry,
{
if (flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET))
return 0;
+ if (flags & LOOKUP_OPEN)
+ /* close-to-open semantics require we go to the server
+ * on each open. By invalidating the dentry we
+ * also ensure nfs_atomic_open() always has exclusive
+ * access to the dentry.
+ */
+ return 1;
if (NFS_SERVER(dir)->flags & NFS_MOUNT_LOOKUP_CACHE_NONEG)
return 1;
/* Case insensitive server? Revalidate negative dentries */
@@ -2113,7 +2120,6 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
struct inode *inode;
unsigned int lookup_flags = 0;
unsigned long dir_verifier;
- bool switched = false;
int created = 0;
int err;
@@ -2158,17 +2164,6 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
attr.ia_size = 0;
}
- if (!(open_flags & O_CREAT) && !d_in_lookup(dentry)) {
- d_drop(dentry);
- switched = true;
- dentry = d_alloc_parallel(dentry->d_parent,
- &dentry->d_name);
- if (IS_ERR(dentry))
- return PTR_ERR(dentry);
- if (unlikely(!d_in_lookup(dentry)))
- return finish_no_open(file, dentry);
- }
-
ctx = create_nfs_open_context(dentry, open_flags, file);
err = PTR_ERR(ctx);
if (IS_ERR(ctx))
@@ -2219,10 +2214,6 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
trace_nfs_atomic_open_exit(dir, ctx, open_flags, err);
put_nfs_open_context(ctx);
out:
- if (unlikely(switched)) {
- d_lookup_done(dentry);
- dput(dentry);
- }
return err;
no_open:
@@ -2245,13 +2236,6 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
res = ERR_PTR(-EOPENSTALE);
}
}
- if (switched) {
- d_lookup_done(dentry);
- if (!res)
- res = dentry;
- else
- dput(dentry);
- }
return finish_no_open(file, res);
}
EXPORT_SYMBOL_GPL(nfs_atomic_open);
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 4/9] nfs: use d_splice_alias() in nfs_link()
2026-10-23 1:50 [PATCH v3 0/9] nfs: prepare for locking changes NeilBrown
` (2 preceding siblings ...)
2026-10-23 1:50 ` [PATCH v3 3/9] nfs: remove d_drop()/d_alloc_parallel() from nfs_atomic_open() NeilBrown
@ 2026-10-23 1:50 ` NeilBrown
2026-10-23 1:50 ` [PATCH v3 5/9] nfs: don't d_drop() before d_splice_alias() NeilBrown
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: NeilBrown @ 2026-10-23 1:50 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, Alexander Viro, Christian Brauner
Cc: Jeff Layton, Jan Kara, linux-fsdevel, linux-nfs, linux-kernel
From: NeilBrown <neil@brown.name>
When filename_linkat() calls filename_create() which ultimately
calls ->lookup, the flags LOOKUP_CREATE|LOOKUP_EXCL are passed.
nfs_lookup() treats this as an exclusive create (which it is) and
skips the ->lookup, leaving the dentry unchanged.
Currently that means nfs_link() can get a hashed dentry (if the name was
already in the cache) or an unhashed dentry (if it wasn't). As none of
d_add(), d_instantiate(), d_splice_alias() could handle both of these,
nfs_link() calls d_drop() and then d_add().
Recent changes to d_splice_alias() mean that it *can* work with either
hashed or unhashed dentries. Future changes to locking mean that it
will be unsafe to d_drop() a dentry while an operation (in this case
"link()") is still ongoing.
So change to use d_splice_alias(), and not to d_drop() until an error is
detected (as in that case was can't be sure what is actually on the server).
Also update the comment for nfs_is_exclusive_create() to note that
link(), mkdir(), mknod(), symlink() all appear as exclusive creates.
Those other than link() already used d_splice_alias() via
nfs_add_or_obtain().
Possibly nfs_is_exclusive_create() should return true even with NFSv2
when the request isn't an open (not LOOKUP_OPEN).
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/nfs/dir.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 59981a4c7403..29492c06ebf5 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1570,6 +1570,9 @@ static int nfs_check_verifier(struct inode *dir, struct dentry *dentry,
/*
* Use intent information to check whether or not we're going to do
* an O_EXCL create using this path component.
+ * Note that link(), mkdir(), mknod(), symlink() all appear as
+ * exclusive creation. Regular file creation could be distinguished
+ * with LOOKUP_OPEN.
*/
static int nfs_is_exclusive_create(struct inode *dir, unsigned int flags)
{
@@ -2699,14 +2702,15 @@ nfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry)
old_dentry, dentry);
trace_nfs_link_enter(inode, dir, dentry);
- d_drop(dentry);
if (S_ISREG(inode->i_mode))
nfs_sync_inode(inode);
error = NFS_PROTO(dir)->link(inode, dir, &dentry->d_name);
if (error == 0) {
nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
ihold(inode);
- d_add(dentry, inode);
+ d_splice_alias(inode, dentry);
+ } else {
+ d_drop(dentry);
}
trace_nfs_link_exit(inode, dir, dentry, error);
return error;
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 5/9] nfs: don't d_drop() before d_splice_alias()
2026-10-23 1:50 [PATCH v3 0/9] nfs: prepare for locking changes NeilBrown
` (3 preceding siblings ...)
2026-10-23 1:50 ` [PATCH v3 4/9] nfs: use d_splice_alias() in nfs_link() NeilBrown
@ 2026-10-23 1:50 ` NeilBrown
2026-10-23 1:50 ` [PATCH v3 6/9] nfs: don't d_drop() before d_splice_alias() in atomic_create NeilBrown
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: NeilBrown @ 2026-10-23 1:50 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, Alexander Viro, Christian Brauner
Cc: Jeff Layton, Jan Kara, linux-fsdevel, linux-nfs, linux-kernel
From: NeilBrown <neil@brown.name>
nfs_add_or_obtain() is used, often via nfs_instantiate(), to attach a
newly created inode to the appropriate dentry - or to provide an
alternate dentry.
It has to drop the dentry first, which is problematic for proposed
locking changes.
As d_splice_alias() now works with hashed dentries, the d_drop() is no
longer needed.
However we still d_drop() on error as the status of the name is
uncertain.
_nfs4_open_and_get_state() is only used for files so we should be able to
use d_instantiate(). However as that depends on the server for
correctness, it is safer to stay with the current code pattern and use
d_splice_alias() there too.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/nfs/dir.c | 4 ++--
fs/nfs/nfs4proc.c | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 29492c06ebf5..aaad823370e2 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2346,8 +2346,6 @@ nfs_add_or_obtain(struct dentry *dentry, struct nfs_fh *fhandle,
struct dentry *d;
int error;
- d_drop(dentry);
-
if (fhandle->size == 0) {
error = NFS_PROTO(dir)->lookup(dir, dentry, &dentry->d_name,
fhandle, fattr);
@@ -2365,6 +2363,8 @@ nfs_add_or_obtain(struct dentry *dentry, struct nfs_fh *fhandle,
inode = nfs_fhget(dentry->d_sb, fhandle, fattr);
d = d_splice_alias(inode, dentry);
out:
+ if (IS_ERR(d))
+ d_drop(dentry);
dput(parent);
return d;
out_error:
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 21e5df7e5251..e8ca92b07190 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3113,7 +3113,6 @@ static int _nfs4_open_and_get_state(struct nfs4_opendata *opendata,
nfs_set_verifier(dentry, dir_verifier);
if (d_really_is_negative(dentry)) {
struct dentry *alias;
- d_drop(dentry);
alias = d_splice_alias(igrab(state->inode), dentry);
/* d_splice_alias() can't fail here - it's a non-directory */
if (alias) {
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 6/9] nfs: don't d_drop() before d_splice_alias() in atomic_create.
2026-10-23 1:50 [PATCH v3 0/9] nfs: prepare for locking changes NeilBrown
` (4 preceding siblings ...)
2026-10-23 1:50 ` [PATCH v3 5/9] nfs: don't d_drop() before d_splice_alias() NeilBrown
@ 2026-10-23 1:50 ` NeilBrown
2026-10-23 1:50 ` [PATCH v3 7/9] nfs: Use d_alloc_trylock() in nfs_prime_dcache() NeilBrown
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: NeilBrown @ 2026-10-23 1:50 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, Alexander Viro, Christian Brauner
Cc: Jeff Layton, Jan Kara, linux-fsdevel, linux-nfs, linux-kernel
From: NeilBrown <neil@brown.name>
When atomic_create fails with -ENOENT we currently d_drop() the dentry
and then re-add it (d_splice_alias()) with a NULL inode.
This drop-and-re-add will not work with proposed locking changes.
As d_splice_alias() now supports hashed dentries, we don't need the
d_drop() until it is determined that some other error has occurred.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/nfs/dir.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index aaad823370e2..8a98247b6b2d 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2180,7 +2180,6 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
err = PTR_ERR(inode);
trace_nfs_atomic_open_exit(dir, ctx, open_flags, err);
put_nfs_open_context(ctx);
- d_drop(dentry);
switch (err) {
case -ENOENT:
if (nfs_server_capable(dir, NFS_CAP_CASE_INSENSITIVE))
@@ -2189,7 +2188,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
dir_verifier = nfs_save_change_attribute(dir);
nfs_set_verifier(dentry, dir_verifier);
d_splice_alias(NULL, dentry);
- break;
+ goto out;
case -EISDIR:
case -ENOTDIR:
if (open_flags & __O_REGULAR) {
@@ -2205,6 +2204,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
default:
break;
}
+ d_drop(dentry);
goto out;
}
file->f_mode |= FMODE_CAN_ODIRECT;
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 7/9] nfs: Use d_alloc_trylock() in nfs_prime_dcache()
2026-10-23 1:50 [PATCH v3 0/9] nfs: prepare for locking changes NeilBrown
` (5 preceding siblings ...)
2026-10-23 1:50 ` [PATCH v3 6/9] nfs: don't d_drop() before d_splice_alias() in atomic_create NeilBrown
@ 2026-10-23 1:50 ` NeilBrown
2026-10-23 1:50 ` [PATCH v3 8/9] nfs: use d_alloc_trylock() in silly-rename NeilBrown
2026-10-23 1:50 ` [PATCH v3 9/9] nfs: use d_duplicate() NeilBrown
8 siblings, 0 replies; 10+ messages in thread
From: NeilBrown @ 2026-10-23 1:50 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, Alexander Viro, Christian Brauner
Cc: Jeff Layton, Jan Kara, linux-fsdevel, linux-nfs, linux-kernel
From: NeilBrown <neil@brown.name>
NFS 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(), which removes the need for
calculating the hash or doing a preliminary lookup.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/nfs/dir.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 8a98247b6b2d..49283f533d6e 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -749,15 +749,12 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry,
if (filename.len == 2 && filename.name[1] == '.')
return;
}
- filename.hash = full_name_hash(parent, filename.name, filename.len);
- dentry = d_lookup(parent, &filename);
again:
- if (!dentry) {
- dentry = d_alloc_parallel(parent, &filename);
- if (IS_ERR(dentry))
- return;
- }
+ dentry = d_alloc_trylock(parent, &filename);
+ if (IS_ERR(dentry))
+ return;
+
if (!d_in_lookup(dentry)) {
/* Is there a mountpoint here? If so, just exit */
if (!nfs_fsid_equal(&NFS_SB(dentry->d_sb)->fsid,
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 8/9] nfs: use d_alloc_trylock() in silly-rename
2026-10-23 1:50 [PATCH v3 0/9] nfs: prepare for locking changes NeilBrown
` (6 preceding siblings ...)
2026-10-23 1:50 ` [PATCH v3 7/9] nfs: Use d_alloc_trylock() in nfs_prime_dcache() NeilBrown
@ 2026-10-23 1:50 ` NeilBrown
2026-10-23 1:50 ` [PATCH v3 9/9] nfs: use d_duplicate() NeilBrown
8 siblings, 0 replies; 10+ messages in thread
From: NeilBrown @ 2026-10-23 1:50 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, Alexander Viro, Christian Brauner
Cc: Jeff Layton, Jan Kara, linux-fsdevel, linux-nfs, linux-kernel
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 then open-code the rest of lookup_slow() to see if it
is free on the server.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/nfs/unlink.c | 59 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 39 insertions(+), 20 deletions(-)
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 11a46a993e11..0eddc7ee1115 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -451,7 +451,7 @@ nfs_sillyrename(struct inode *dir, struct dentry *dentry)
static unsigned int sillycounter;
unsigned char silly[SILLYNAME_LEN + 1];
unsigned long long fileid;
- struct dentry *sdentry;
+ struct dentry *sdentry, *old;
struct inode *inode = d_inode(dentry);
struct rpc_task *task;
int error = -EBUSY;
@@ -468,26 +468,45 @@ nfs_sillyrename(struct inode *dir, struct dentry *dentry)
fileid = d_inode(dentry)->i_ino;
- sdentry = NULL;
- do {
+newname:
+ sillycounter++;
+ scnprintf(silly, sizeof(silly),
+ SILLYNAME_PREFIX "%0*llx%0*x",
+ SILLYNAME_FILEID_LEN, fileid,
+ SILLYNAME_COUNTER_LEN, sillycounter);
+
+ dfprintk(VFS, "NFS: trying to rename %pd to %s\n", dentry, silly);
+ sdentry = d_alloc_trylock(dentry->d_parent, &QSTR(silly));
+ if (sdentry == ERR_PTR(-EWOULDBLOCK))
+ /* Name currently being looked up */
+ goto newname;
+ if (IS_ERR(sdentry))
+ goto out;
+ if (!d_in_lookup(sdentry)) {
+ if (d_really_is_negative(sdentry)) {
+ /* try to get an in-lookup dentry */
+ d_drop(sdentry);
+ sillycounter--;
+ }
dput(sdentry);
- sillycounter++;
- scnprintf(silly, sizeof(silly),
- SILLYNAME_PREFIX "%0*llx%0*x",
- SILLYNAME_FILEID_LEN, fileid,
- SILLYNAME_COUNTER_LEN, sillycounter);
-
- dfprintk(VFS, "NFS: trying to rename %pd to %s\n",
- dentry, silly);
-
- sdentry = lookup_noperm(&QSTR(silly), dentry->d_parent);
- /*
- * N.B. Better to return EBUSY here ... it could be
- * dangerous to delete the file while it's in use.
- */
- if (IS_ERR(sdentry))
- goto out;
- } while (d_inode(sdentry) != NULL); /* need negative lookup */
+ goto newname;
+ }
+ /* This name isn't known locally - check on server */
+ old = nfs_lookup(dir, sdentry, 0);
+ d_lookup_done(sdentry);
+ /*
+ * N.B. Better to return EBUSY here ... it could be
+ * dangerous to delete the file while it's in use.
+ */
+ if (IS_ERR(old)) {
+ dput(sdentry);
+ goto out;
+ }
+ if (old || d_is_positive(sdentry)) {
+ dput(old);
+ dput(sdentry);
+ goto newname;
+ }
ihold(inode);
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 9/9] nfs: use d_duplicate()
2026-10-23 1:50 [PATCH v3 0/9] nfs: prepare for locking changes NeilBrown
` (7 preceding siblings ...)
2026-10-23 1:50 ` [PATCH v3 8/9] nfs: use d_alloc_trylock() in silly-rename NeilBrown
@ 2026-10-23 1:50 ` NeilBrown
8 siblings, 0 replies; 10+ messages in thread
From: NeilBrown @ 2026-10-23 1:50 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, Alexander Viro, Christian Brauner
Cc: Jeff Layton, Jan Kara, linux-fsdevel, linux-nfs, linux-kernel
From: NeilBrown <neil@brown.name>
As preparation for d_alloc_parallel() being allowed without the
directory locked, use d_duplicate() to duplicate a dentry for silly
rename.
This results in an in-lookup name, so d_lookup_done() must be called.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/nfs/dir.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 49283f533d6e..f02b1b068d93 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2802,11 +2802,12 @@ int nfs_rename(const struct mnt_idmap *idmap, struct inode *old_dir,
spin_unlock(&new_dentry->d_lock);
/* copy the target dentry's name */
- dentry = d_alloc(new_dentry->d_parent,
- &new_dentry->d_name);
- if (!dentry)
+ dentry = d_duplicate(new_dentry);
+ if (IS_ERR(dentry)) {
+ /* Keep using -ETXTBSY error */
+ dentry = NULL;
goto out;
-
+ }
/* silly-rename the existing target ... */
err = nfs_sillyrename(new_dir, new_dentry);
if (err)
@@ -2871,8 +2872,10 @@ int nfs_rename(const struct mnt_idmap *idmap, struct inode *old_dir,
nfs_dentry_handle_enoent(old_dentry);
/* new dentry created? */
- if (dentry)
+ if (dentry) {
+ d_lookup_done(dentry);
dput(dentry);
+ }
return error;
}
EXPORT_SYMBOL_GPL(nfs_rename);
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply [flat|nested] 10+ messages in thread