mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Zhan Xusheng <zhanxusheng1024@gmail.com>
To: cel@kernel.org, jlayton@kernel.org
Cc: neil@brown.name, okorniev@redhat.com, Dai.Ngo@oracle.com,
	tom@talpey.com, linux-nfs@vger.kernel.org,
	linux-kernel@vger.kernel.org, zhanxusheng@xiaomi.com
Subject: [PATCH v2] nfsd: don't block on a file lease while holding the parent lock
Date: Wed, 11 Nov 2026 15:48:35 +0800	[thread overview]
Message-ID: <20261111074835.599584-1-zhanxusheng1024@gmail.com> (raw)

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.

Guarded creates and exclusive creates with a different verifier
fail with NFS4ERR_EXIST on an existing file and have no use for the
open. Skip it for them, as nfsd did before that commit, so that they
don't break the lease and, with O_NONBLOCK, don't return
NFS4ERR_DELAY instead of NFS4ERR_EXIST.

Fixes: 2a60db3525d4 ("nfsd: (almost) always open file in nfsd4_create_file()")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
v2: also skip the open for guarded and non-matching exclusive
creates (sashiko, via Jeff). The other part of that report, the open
running before the permission check, is older than this patch and is
not changed here.

QEMU guest on b3dfbde62cf1, a local write lease held on f:

                  touch in f's dir while    open(O_CREAT|O_EXCL)
                  "echo x >> f" waits       of f
  b3dfbde62cf1    13.0s                     EEXIST after 15.3s
  + v1            20ms                      EEXIST after 26.5s
  + v2            10ms                      EEXIST after 2ms

v1: https://lore.kernel.org/all/20261110080038.232210-1-zhanxusheng1024@gmail.com/

 fs/nfsd/nfs4proc.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 3a82af381a8d..f19bd69d66f4 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -250,10 +250,21 @@ static inline bool nfsd4_create_is_exclusive(int createmode)
 		createmode == NFS4_CREATE_EXCLUSIVE4_1;
 }
 
+/* Does @child carry the verifier of an earlier exclusive create? */
+static bool nfsd4_create_verf_matches(struct dentry *child, const u32 *verf)
+{
+	struct inode *inode = d_inode(child);
+
+	return inode_get_mtime_sec(inode) == verf[0] &&
+	       inode_get_atime_sec(inode) == verf[1] &&
+	       inode->i_size == 0;
+}
+
 static struct file *do_lookup_open(struct path *parent,
 				   struct qstr *name,
 				   unsigned int oflags,
-				   umode_t mode)
+				   umode_t mode,
+				   const u32 *verf)
 {
 	struct file *filp = NULL;
 	struct path path;
@@ -277,8 +288,16 @@ static struct file *do_lookup_open(struct path *parent,
 		 */
 		int err = nfsd_check_obj_isreg(child);
 
+		/*
+		 * Guarded creates and exclusive creates with another
+		 * verifier fail with EEXIST. Don't open the file for them,
+		 * that would only break a lease.
+		 */
 		if (err)
 			filp = ERR_PTR(err);
+		else if ((oflags & O_EXCL) ||
+			 (verf && !nfsd4_create_verf_matches(child, verf)))
+			filp = ERR_PTR(-EEXIST);
 		else
 			filp = dentry_open(&path, oflags, current_cred());
 	} else if (!(oflags & O_CREAT)) {
@@ -312,13 +331,13 @@ 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,
 		.dentry = fhp->fh_dentry,
 	};
-	__u32 v_mtime, v_atime;
+	u32 verf[2] = {};
 	__be32 status, create_status;
 	int want_write_err;
 
@@ -395,8 +414,6 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		attrs.na_pacl = posix_acl_dup(open->op_pacl);
 	}
 
-	v_mtime = 0;
-	v_atime = 0;
 	if (nfsd4_create_is_exclusive(open->op_createmode)) {
 		u32 *verifier = (u32 *)open->op_verf.data;
 
@@ -408,13 +425,13 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		 * verifier, then do_open_lookup() will also need to be
 		 * fixed accordingly.
 		 */
-		v_mtime = verifier[0] & 0x7fffffff;
-		v_atime = verifier[1] & 0x7fffffff;
+		verf[0] = verifier[0] & 0x7fffffff;
+		verf[1] = verifier[1] & 0x7fffffff;
 
 		iap->ia_valid |= ATTR_MTIME | ATTR_ATIME |
 				 ATTR_MTIME_SET|ATTR_ATIME_SET;
-		iap->ia_mtime.tv_sec = v_mtime;
-		iap->ia_atime.tv_sec = v_atime;
+		iap->ia_mtime.tv_sec = verf[0];
+		iap->ia_atime.tv_sec = verf[1];
 		iap->ia_mtime.tv_nsec = 0;
 		iap->ia_atime.tv_nsec = 0;
 	}
@@ -428,7 +445,9 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 				       &QSTR_LEN(open->op_fname,
 						 open->op_fnamelen),
 				       oflags,
-				       open->op_iattr.ia_mode);
+				       open->op_iattr.ia_mode,
+				       nfsd4_create_is_exclusive(open->op_createmode) ?
+				       verf : NULL);
 	if (IS_ERR(open->op_filp)) {
 		int hosterr = PTR_ERR(open->op_filp);
 
@@ -456,9 +475,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 
 	if (!open->op_created &&
 	    nfsd4_create_is_exclusive(open->op_createmode) &&
-	    inode_get_mtime_sec(d_inode(child)) == v_mtime &&
-	    inode_get_atime_sec(d_inode(child)) == v_atime &&
-	    d_inode(child)->i_size == 0)
+	    nfsd4_create_verf_matches(child, verf))
 		open->op_created = true;
 
 	if (!open->op_created) {
-- 
2.43.0


                 reply	other threads:[~2026-11-11  7:48 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261111074835.599584-1-zhanxusheng1024@gmail.com \
    --to=zhanxusheng1024@gmail.com \
    --cc=Dai.Ngo@oracle.com \
    --cc=cel@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    --cc=zhanxusheng@xiaomi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®