[PATCH v3 2/7] nfsd: answer NFS4ERR_DELAY instead of waiting for a layout recall

[Daejun Park via B4 Relay]

From: Daejun Park <daejun7.park@xxxxxxxxxxx>

A WRITE, a SETATTR that changes the size (also an OPEN that truncates
and an NFSv2 or NFSv3 CREATE of an existing file with a size), an
ALLOCATE, a DEALLOCATE, a CLONE and a COPY make XFS recall the layouts
of the file and wait in __break_lease() until they are returned or the
lease break times out: xfs_break_leased_layouts(), and
xfs_iolock_two_inodes_and_break_layout() for both files of a clone. The
nfsd thread serving the request sleeps there too. The client decides
when it returns its layout, so a few such requests can occupy every nfsd
thread, and the server then cannot process the LAYOUTRETURN that would
release them.

Before calling into the filesystem, recall the layouts without waiting,
as nfsd_open_break_lease() does for delegations, and return
-EWOULDBLOCK while a layout is held, which nfserrno() maps to
NFS4ERR_DELAY (NFS3ERR_JUKEBOX for NFSv3), so that the client retries.
When this starts the recall of a layout of another client, first give
the layouts NFSD_LAYOUTRETURN_TIMEOUT to come back, the 30 ms that
nfsd_wait_for_delegreturn() allows a delegation: a client that is not
doing I/O through its layout returns it within a few milliseconds, and
the retry would cost the client of the request at least 100 ms. Do not
wait for a recall that is already under way, nor when the client of
the request holds a layout of the file itself: it returns that layout
only after its own I/O through it has completed. nfsd4_close_layout()
wakes the waiter when a layout lease is removed. For a COPY or a CLONE,
both recalls are started before the wait. For a SETATTR, the recall
comes before inode_lock, which the LAYOUTCOMMIT that a client sends
before its LAYOUTRETURN takes. A file without leases costs one lockless
check of its lease list; a file with other leases, such as
delegations, a walk of that list under flc_lock.

Answering NFS4ERR_DELAY here was discussed in 2023 (second Link).
Passing a non-blocking flag down to the break in the filesystem would
need VFS and XFS changes and a way through notify_change(),
vfs_fallocate() and the remap path; breaking before the call, as
nfsd_open_break_lease() does for delegations, needs neither.

An nfsd thread can still block in the filesystem when a LAYOUTGET
completes between this check and the recall in the filesystem. No layout
lease can be added while a recall is pending, so this takes a file that
had no layout when nfsd checked. A recall that meets a LAYOUTGET in
progress fails that LAYOUTGET (previous patch), so the layout it would
have handed out is not left unrecalled. A COPY recalls the layouts
before it is split into a synchronous and an asynchronous copy, so an
asynchronous COPY gets NFS4ERR_DELAY as well instead of starting a
kthread that waits for the recall. That kthread copies 4 MiB at a time,
and a layout handed out while it runs still makes it wait in the
filesystem. The source of an inter-server COPY is not on this server, so
only its destination is recalled.

With a client that holds a layout of the destination and has its path to
the device cut for 70 seconds, an 8 MiB asynchronous COPY from another
client got NFS4ERR_DELAY 9 times and was done after 84 seconds: the
client holding the layout was fenced at a retry 53 seconds in, and the
COPY went on at a retry 31 seconds later. With v2 of this series, which
recalled only for a synchronous COPY, the copy kthread waited in XFS and
the COPY was done after 48 seconds. Without the cut, both took about 50
ms, and the data was in place in all four runs.

The cost is on a request that conflicts with a layout. When the layout
does not come back within 30 ms, or the client of the request holds it,
the request completes only when the client sends it again. The Linux
client does that after at least 100 ms for NFSv4 (NFS4_POLL_RETRY_MIN)
and after 5 seconds for NFSv3 (NFS_JUKEBOX_RETRY_TIME), where it used to
wait only until the layout was returned. An NFSv2 WRITE has its reply
dropped, as for any NFSERR_JUKEBOX, and an NFSv2 SETATTR or CREATE is
answered as for a delegation conflict. The recall now also comes before
the checks that would fail the request: a COPY recalls the layouts of
its source as well, because the filesystem may do it as a clone, also
where it cannot, and a CLONE on a filesystem without reflink can get
NFS4ERR_DELAY before NFS4ERR_NOTSUPP. An NFSv2 or NFSv3 CREATE of an
existing file with a size does the SETATTR with the parent directory
locked and holds it during the wait.

With 4 nfsd threads, a client that holds a layout of a file and has
lost its path to the device, and another client (NFSv4.0) that sends 8
4 KiB WRITEs to that file through the metadata server while it reads
4 KiB of another file every second: the WRITEs take about 45 seconds
with and without this patch, as they wait for the lease break, and
without it the reads of the other file wait as long. With it the
longest of 84 reads takes 31 ms, and 0.22 seconds with 64 WRITEs. With
the second client on NFSv3, the longest read takes 21 ms instead of 47
seconds, and the WRITEs take 51 seconds instead of 47, as the client
waits 5 seconds after NFS3ERR_JUKEBOX.

A client that writes to a file both through its layout and through the
server recalls its own layout with each such WRITE. About half of its
LAYOUTGETs then meet that recall and get NFS4ERR_RECALLCONFLICT, as
before; the others are in progress when the WRITE breaks the lease, and
the previous patch fails them. The Linux client waits up to 15 seconds
between LAYOUTGETs that fail with NFS4ERR_RECALLCONFLICT. Writing 1 MiB
blocks through its layout and 512 bytes through the server for 30
seconds, such a client wrote 48, 46 and 4 blocks through the layout in
three runs, against 534, 537 and 505 without this series. There most
recalls of its layout ended in the failure path, and 7, 4 and 20 of the
blocks read back as zeroes after a successful fsync(); none did with
this series.

Writes to a file without leases are not changed: fio with O_DIRECT,
NFSv4.0, 10 runs of 30 seconds, without KASAN and lockdep, gave median
65.8 and 67.5 MiB/s for 1 MiB sequential writes without and with an
earlier revision of this series (the same path for such a file), and 262
and 263 IOPS for 4 KiB random writes.

Fixes: c5c707f96fc9 ("nfsd: implement pNFS layout recalls")
Suggested-by: Chuck Lever <cel@xxxxxxxxxx>
Link: https://lore.kernel.org/r/63ef8d7c-086f-4cc8-84bb-72c1b23bf596@slotpi15m67
Link: https://lore.kernel.org/r/ZVdj2U+kJ7DlI22g@xxxxxxxxxxxxx
Signed-off-by: Daejun Park <daejun7.park@xxxxxxxxxxx>
---
 fs/nfsd/nfs4layouts.c | 57 ++++++++++++++++++++++++++++++++++++-
 fs/nfsd/nfs4proc.c    | 10 +++++++
 fs/nfsd/pnfs.h        | 11 ++++++++
 fs/nfsd/state.h       |  1 +
 fs/nfsd/vfs.c         | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/nfsd/vfs.h         |  1 +
 6 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
index c33935f486..1f06478056 100644
--- a/fs/nfsd/nfs4layouts.c
+++ b/fs/nfsd/nfs4layouts.c
@@ -153,13 +153,68 @@ void nfsd4_close_layout(struct nfs4_layout_stateid *ls)
 	spin_unlock(&ls->ls_stid.sc_file->fi_lock);
 
 	if (fl) {
-		if (!nfsd4_layout_ops[ls->ls_layout_type]->disable_recalls)
+		if (!nfsd4_layout_ops[ls->ls_layout_type]->disable_recalls) {
 			kernel_setlease(fl->nf_file, F_UNLCK, NULL,
 					(void **)&ls);
+			/*
+			 * For nfsd_break_layouts(). No barrier needed: its
+			 * waiter rechecks under flc_lock, which removing the
+			 * lease took.
+			 */
+			wake_up_var(file_inode(fl->nf_file));
+		}
 		nfsd_file_put(fl);
 	}
 }
 
+/**
+ * nfsd4_layout_holders - Report which clients hold layout leases on a file
+ * @inode: file whose layout leases are checked
+ *
+ * Context: Takes the flc_lock of @inode.
+ *
+ * Return: a mask of NFSD_LAYOUTS_OWN if the client of the current request
+ * holds a layout lease on @inode, NFSD_LAYOUTS_OTHER if another client
+ * holds one, and NFSD_LAYOUTS_IDLE if a lease of another client is not
+ * being broken yet.
+ */
+unsigned int nfsd4_layout_holders(struct inode *inode)
+{
+	struct file_lock_context *ctx = locks_inode_context(inode);
+	struct nfs4_client *clp = NULL;
+	unsigned int holders = 0;
+	struct svc_rqst *rqstp;
+	struct file_lease *fl;
+
+	if (!ctx || list_empty_careful(&ctx->flc_lease))
+		return 0;
+	/* Without a compound, as for NFSv3, every layout is another client's */
+	rqstp = nfsd_current_rqst();
+	if (nfsd_v4client(rqstp)) {
+		struct nfsd_thread_local_info *ntli = rqstp->rq_private;
+
+		clp = *ntli->ntli_lease_breaker;
+	}
+
+	spin_lock(&ctx->flc_lock);
+	for_each_file_lock(fl, &ctx->flc_lease) {
+		struct nfs4_layout_stateid *ls = fl->c.flc_owner;
+
+		if (fl->fl_lmops != &nfsd4_layouts_lm_ops)
+			continue;
+		if (clp && ls->ls_stid.sc_client == clp) {
+			holders |= NFSD_LAYOUTS_OWN;
+			continue;
+		}
+		holders |= NFSD_LAYOUTS_OTHER;
+		if (!(fl->c.flc_flags &
+		      (FL_UNLOCK_PENDING | FL_DOWNGRADE_PENDING)))
+			holders |= NFSD_LAYOUTS_IDLE;
+	}
+	spin_unlock(&ctx->flc_lock);
+	return holders;
+}
+
 static void
 nfsd4_free_layout_stateid(struct nfs4_stid *stid)
 {
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 0848baad68..a50f9dcb3a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2298,6 +2298,7 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfsd4_async_copy *async_copy = NULL;
 	struct nfsd4_copy *copy = &u->copy;
 	struct nfsd42_write_res *result;
+	struct inode *src, *dst;
 	__be32 status;
 
 	result = &copy->cp_res;
@@ -2322,6 +2323,15 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 			goto out;
 	}
 
+	/*
+	 * A copy can be done as a clone, which breaks both files. The source
+	 * of an inter-server copy is not local.
+	 */
+	dst = file_inode(copy->nf_dst->nf_file);
+	src = copy->nf_src ? file_inode(copy->nf_src->nf_file) : dst;
+	if (nfsd_break_layouts(dst, src != dst ? src : NULL))
+		goto out_err;
+
 	memcpy(&copy->fh, &cstate->current_fh.fh_handle,
 		sizeof(struct knfsd_fh));
 	if (nfsd4_copy_is_async(copy)) {
diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
index 9b0f17a4d1..e5f2e11c88 100644
--- a/fs/nfsd/pnfs.h
+++ b/fs/nfsd/pnfs.h
@@ -72,6 +72,11 @@ int nfsd4_set_deviceid(struct nfsd4_deviceid *id, const struct svc_fh *fhp,
 struct nfsd4_deviceid_map *nfsd4_find_devid_map(int idx);
 #endif /* CONFIG_NFSD_V4 */
 
+/* The mask that nfsd4_layout_holders() returns */
+#define NFSD_LAYOUTS_OWN	0x1
+#define NFSD_LAYOUTS_OTHER	0x2
+#define NFSD_LAYOUTS_IDLE	0x4
+
 #ifdef CONFIG_NFSD_PNFS
 void nfsd4_setup_layout_type(struct svc_export *exp);
 void nfsd4_return_all_client_layouts(struct nfs4_client *);
@@ -79,6 +84,7 @@ void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls);
 void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
 		struct nfs4_file *fp);
 void nfsd4_close_layout(struct nfs4_layout_stateid *ls);
+unsigned int nfsd4_layout_holders(struct inode *inode);
 void nfsd4_stop_layout_fence(struct nfs4_layout_stateid *ls);
 int nfsd4_init_pnfs(void);
 void nfsd4_exit_pnfs(void);
@@ -107,6 +113,11 @@ static inline void nfsd4_close_layout(struct nfs4_layout_stateid *ls)
 {
 }
 
+static inline unsigned int nfsd4_layout_holders(struct inode *inode)
+{
+	return 0;
+}
+
 static inline void nfsd4_stop_layout_fence(struct nfs4_layout_stateid *ls)
 {
 }
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 20ab27221c..b1f4c167ea 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -62,6 +62,7 @@
 #define	NFS4_CLIENTS_PER_GB		1024
 #define NFSD_DELEGRETURN_TIMEOUT	(HZ / 34)	/* 30ms */
 #define	NFSD_CB_GETATTR_TIMEOUT		NFSD_DELEGRETURN_TIMEOUT
+#define	NFSD_LAYOUTRETURN_TIMEOUT	NFSD_DELEGRETURN_TIMEOUT
 
 typedef struct {
 	u32             cl_boot;
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f4069da853..c4b32e41b4 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -46,6 +46,7 @@
 #include "netns.h"
 #include "stats.h"
 #include "vfs.h"
+#include "pnfs.h"
 #include "filecache.h"
 #include "trace.h"
 
@@ -583,6 +584,15 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		err = nfsd_get_write_access(fhp);
 		if (err)
 			return err;
+		/*
+		 * Not under inode_lock: the LAYOUTCOMMIT that a client sends
+		 * before it returns its layout takes it.
+		 */
+		err = nfserrno(nfsd_break_layouts(inode, NULL));
+		if (err) {
+			put_write_access(inode);
+			return err;
+		}
 	}
 
 	inode_lock(inode);
@@ -721,6 +731,14 @@ int nfsd_clone_file_range(struct file *src, u64 src_pos, struct file *dst,
 			  u64 dst_pos, u64 count, errseq_t *since)
 {
 	loff_t cloned;
+	int ret;
+
+	/* The filesystem breaks the layouts of both files */
+	ret = nfsd_break_layouts(file_inode(src),
+				 file_inode(dst) != file_inode(src) ?
+				 file_inode(dst) : NULL);
+	if (ret)
+		return ret;
 
 	*since = READ_ONCE(dst->f_wb_err);
 	cloned = vfs_clone_file_range(src, src_pos, dst, dst_pos, count, 0);
@@ -794,7 +812,9 @@ __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	if (!S_ISREG(file_inode(file)->i_mode))
 		return nfserr_inval;
 
-	error = vfs_fallocate(file, flags, offset, len);
+	error = nfsd_break_layouts(file_inode(file), NULL);
+	if (!error)
+		error = vfs_fallocate(file, flags, offset, len);
 	if (!error)
 		error = commit_metadata(fhp);
 
@@ -881,6 +901,58 @@ int nfsd_open_break_lease(struct inode *inode, int access)
 	return break_lease(inode, mode | O_NONBLOCK);
 }
 
+static bool nfsd_other_layouts(struct inode *inode)
+{
+	return nfsd4_layout_holders(inode) & NFSD_LAYOUTS_OTHER;
+}
+
+/**
+ * nfsd_break_layouts - Recall the layouts of one or two files
+ * @inode: file that is about to be modified
+ * @inode2: the other file of a COPY or CLONE, or NULL
+ *
+ * Context: Process context. Sleeps for up to NFSD_LAYOUTRETURN_TIMEOUT.
+ *
+ * Return: zero if no layout is left, -EWOULDBLOCK while a recall is under
+ * way, or -ENOMEM
+ */
+int nfsd_break_layouts(struct inode *inode, struct inode *inode2)
+{
+	long timeout = NFSD_LAYOUTRETURN_TIMEOUT;
+	unsigned int holders;
+	int ret, ret2 = 0;
+
+	holders = nfsd4_layout_holders(inode);
+	if (inode2)
+		holders |= nfsd4_layout_holders(inode2);
+	if (!holders)
+		return 0;
+
+	/* Start the recalls without waiting, unlike the filesystem's break */
+	ret = break_layout(inode, false);
+	if (inode2)
+		ret2 = break_layout(inode2, false);
+	if (ret != -EWOULDBLOCK && ret2 != -EWOULDBLOCK)
+		return ret ? ret : ret2;
+	/*
+	 * Briefly wait for other clients' layouts if this started their
+	 * recall, as for delegations. The client of this request returns its
+	 * own only after its I/O through them.
+	 */
+	if ((holders & NFSD_LAYOUTS_OWN) || !(holders & NFSD_LAYOUTS_IDLE))
+		return -EWOULDBLOCK;
+
+	timeout = wait_var_event_timeout(inode, !nfsd_other_layouts(inode),
+					 timeout);
+	if (inode2 && timeout)
+		wait_var_event_timeout(inode2, !nfsd_other_layouts(inode2),
+				       timeout);
+	ret = break_layout(inode, false);
+	if (!ret && inode2)
+		ret = break_layout(inode2, false);
+	return ret;
+}
+
 /*
  * Open an existing file or directory.
  * The may_flags argument indicates the type of open (read/write/lock)
@@ -1490,6 +1562,10 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		goto out_nfserr;
 	}
 
+	host_err = nfsd_break_layouts(file_inode(file), NULL);
+	if (host_err)
+		goto out_nfserr;
+
 	since = READ_ONCE(file->f_wb_err);
 	if (verf)
 		nfsd_copy_write_verifier(verf, nn);
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index eb75e4c823..1f05c83a3e 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -171,6 +171,7 @@ __be32		nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 			    char *name, void *buf, u32 len, u32 flags);
 #endif
 int 		nfsd_open_break_lease(struct inode *, int);
+int		nfsd_break_layouts(struct inode *inode, struct inode *inode2);
 __be32		nfsd_open(struct svc_rqst *, struct svc_fh *, umode_t,
 				int, struct file **);
 int		nfsd_open_verified(struct svc_fh *fhp, umode_t type, int may_flags,

-- 
2.43.0