[PATCH] nfsd: don't block on a file lease while holding the parent lock

[PATCH] nfsd: don't block on a file lease while holding the parent lock

[Zhan Xusheng]

From: Zhan Xusheng <zhanxusheng@xiaomi.com>

Since commit 2a60db3525d4 ("nfsd: (almost) always open file in
nfsd4_create_file()"), an NFSv4 OPEN with create that finds an
existing file opens it with dentry_open() while start_creating()
still holds the parent directory lock. The open flags lack
O_NONBLOCK, so break_lease() in do_dentry_open() blocks. Before that
commit the file was opened later by __nfsd_open(), which breaks
leases with O_NONBLOCK, and the client retried on NFS4ERR_DELAY.

A lease on the file now stalls an nfsd thread for up to
lease-break-time with the directory locked, and other creates,
unlinks and renames in that directory wait for it.

To reproduce, set fs.lease-break-time=15, take F_SETLEASE F_WRLCK on
/srv/g from a local process and keep it, and run "echo x >> /mnt/g"
on an NFSv4.2 mount of /srv from localhost. The nfsd thread blocks
in break_lease() for 15s, and "touch /srv/other" on the server takes
13s.

Pass O_NONBLOCK so that break_lease() returns -EWOULDBLOCK and the
client gets NFS4ERR_DELAY again.

Fixes: 2a60db3525d4 ("nfsd: (almost) always open file in nfsd4_create_file()")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
With this patch the same test hits break_lease_noblock and the touch
takes 20-30ms (QEMU guest, f66469d9905b).

NeilBrown's "nfsd: switch NFS4 OPEN to use vfs_lookup_open()" (v2
12/14) makes the same change; that series is not merged.

 fs/nfsd/nfs4proc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 3a82af381a8d..e95258b44c72 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -312,7 +312,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		.na_iattr	= iap,
 		.na_seclabel	= &open->op_label,
 	};
-	int oflags = O_CREAT | O_LARGEFILE;
+	int oflags = O_CREAT | O_LARGEFILE | O_NONBLOCK;
 	struct dentry *child = ERR_PTR(-EINVAL);
 	struct path parent = {
 		.mnt = fhp->fh_export->ex_path.mnt,
-- 
2.43.0

Re: [PATCH] nfsd: don't block on a file lease while holding the parent lock

[Jeff Layton]

On Tue, 2026-11-10 at 16:00 +0800, Zhan Xusheng wrote:
> From: Zhan Xusheng <zhanxusheng@xiaomi.com>
> 
> Since commit 2a60db3525d4 ("nfsd: (almost) always open file in
> nfsd4_create_file()"), an NFSv4 OPEN with create that finds an
> existing file opens it with dentry_open() while start_creating()
> still holds the parent directory lock. The open flags lack
> O_NONBLOCK, so break_lease() in do_dentry_open() blocks. Before that
> commit the file was opened later by __nfsd_open(), which breaks
> leases with O_NONBLOCK, and the client retried on NFS4ERR_DELAY.
> 
> A lease on the file now stalls an nfsd thread for up to
> lease-break-time with the directory locked, and other creates,
> unlinks and renames in that directory wait for it.
> 
> To reproduce, set fs.lease-break-time=15, take F_SETLEASE F_WRLCK on
> /srv/g from a local process and keep it, and run "echo x >> /mnt/g"
> on an NFSv4.2 mount of /srv from localhost. The nfsd thread blocks
> in break_lease() for 15s, and "touch /srv/other" on the server takes
> 13s.
> 
> Pass O_NONBLOCK so that break_lease() returns -EWOULDBLOCK and the
> client gets NFS4ERR_DELAY again.
> 
> Fixes: 2a60db3525d4 ("nfsd: (almost) always open file in nfsd4_create_file()")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
> With this patch the same test hits break_lease_noblock and the touch
> takes 20-30ms (QEMU guest, f66469d9905b).
> 
> NeilBrown's "nfsd: switch NFS4 OPEN to use vfs_lookup_open()" (v2
> 12/14) makes the same change; that series is not merged.
> 
>  fs/nfsd/nfs4proc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 3a82af381a8d..e95258b44c72 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -312,7 +312,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		.na_iattr	= iap,
>  		.na_seclabel	= &open->op_label,
>  	};
> -	int oflags = O_CREAT | O_LARGEFILE;
> +	int oflags = O_CREAT | O_LARGEFILE | O_NONBLOCK;
>  	struct dentry *child = ERR_PTR(-EINVAL);
>  	struct path parent = {
>  		.mnt = fhp->fh_export->ex_path.mnt,

Looks reasonable, but Sashiko has a (valid) comment:

https://sashiko.dev/#/patchset/20261110080038.232210-1-zhanxusheng1024%40gmail.com

Can you either rebut that or send an updated version?

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>