From: Jeff Layton <jlayton@kernel.org>
To: daejun7.park@samsung.com, Chuck Lever <cel@kernel.org>,
NeilBrown <neil@brown.name>,
Olga Kornievskaia <okorniev@redhat.com>,
Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Cc: Christoph Hellwig <hch@lst.de>,
Benjamin Coddington <bcodding@redhat.com>,
Trond Myklebust <trondmy@kernel.org>,
Anna Schumaker <anna@kernel.org>,
linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/7] nfsd: layout recall and lease break fixes
Date: Wed, 14 Oct 2026 07:26:03 -0400 [thread overview]
Message-ID: <46ba944d8dcb989c60c276813339dcb8564a823f.camel@kernel.org> (raw)
In-Reply-To: <20261014-nfsd-layout-recall-v2-0-764462bddfd1@samsung.com>
On Wed, 2026-10-14 at 16:29 +0900, Daejun Park via B4 Relay wrote:
> nfsd polls a client that has accepted a CB_LAYOUTRECALL by sending the
> callback again every 10 ms, with the stateid of the first callback. A
> Linux client from v6.8 on answers the repeat with NFS4ERR_OLD_STATEID,
> and nfsd takes that as a failed recall. So a client that needs more
> than 10 ms to return a layout has the layout revoked and the fence path
> run. With the block layout that client loses data it has written and
> synced, with the SCSI layout it is cut off from the device. Patch 5
> lets nfsd keep polling instead.
>
> A recall that takes longer makes the operation that started it wait
> longer. When that operation is a WRITE, a SETATTR that changes the
> size, an ALLOCATE, a DEALLOCATE, a CLONE or a COPY, the nfsd thread
> serving it waits in XFS until the layouts are returned or the lease
> break times out. As Chuck pointed out on v1, a few such requests can
> take every nfsd thread, and the server then cannot serve the
> LAYOUTRETURN that would release them. Patch 2 recalls the layouts
> before the operation without waiting, gives the layouts of other
> clients 30 ms to come back when it starts their recall, and otherwise
> answers NFS4ERR_DELAY.
>
> The cost of patch 2: a client that writes to a file through its layout
> and, for writes that are not aligned, through the server recalls its
> own layout with each such WRITE and has its LAYOUTGETs fail with
> NFS4ERR_RECALLCONFLICT meanwhile. In test 2 it wrote 48, 46 and 4 1 MiB
> blocks through its layout in 30 seconds instead of 534, 537 and 505;
> before, 6 of 7, 4 of 4 and 17 of 20 recalls ended in the failure path
> and 7, 4 and 20 blocks were lost. The series on top of this one, with
> the NFS client fix it needs, brings that to 193, 192 and 184 blocks.
>
> Patches 3 and 4 are the other two that patch 5 needs. Patch 3: the
> limit of two lease periods on the poll has never triggered, and without
> it the poll would have no end. Patch 4: for the block layout a lease
> break gives up after fs.lease-break-time (45 s) without running
> /sbin/nfsd-recall-failed. Until now the recall had failed long before
> that; with patch 5 it is polled for two lease periods (180 s). Patch 4
> fences the client at fs.lease-break-time, as the SCSI layout does since
> v7.1; by default that is shorter than the one lease period RFC 8881
> section 12.5.5 has the server wait after a recall.
>
> Patch 1 fixes a lease break that hits a LAYOUTGET in progress and then
> waits for a layout that is never recalled. It comes first because
> patch 2 makes that common: the break of patch 2 runs outside the XFS
> iolock, so it can land between the allocation of a layout stateid and
> the insertion of its first layout.
>
> Patches 6 and 7 deal with two answers that the polls handle badly. A
> client that a LAYOUTRETURN has moved past the seqid of the recall
> answers every poll NFS4ERR_OLD_STATEID and is never asked for the
> layouts it still holds; patch 6 polls such a client, one that has not
> answered NFS4_OK, with a new seqid. A client that answers
> NFS4ERR_BAD_STATEID after returning its last layout is fenced; patch 7
> looks for layouts first.
>
> Patches 1 to 5 have to go together, also to stable kernels. Patch 6
> builds on patches 1 and 5, and patch 7 stands on its own. If patch 2
> should not go to stable, patches 3 to 5 should not either; patches 1
> and 7 can go alone. Kernels before v7.1 do not have the lease break
> timeout that patch 4 fixes; there an operation on the server that does
> not come from nfsd waits for the recall for up to two lease periods, as
> it already does, without a limit, for a client that keeps answering
> NFS4ERR_DELAY. Patch 2 uses nfsd_thread_local_info and needs adapting
> where that is missing.
>
> Tests. QEMU guests on one host: an nvmet-tcp target, the server (nfsd,
> XFS on the nvmet namespace, pnfs export) and up to three clients, all
> on the same kernel with KASAN and lockdep, NFS v4.2, block layout
> unless noted, O_DIRECT I/O. "v2" is this series on nfsd-testing
> 56589cdb5881, "v1" is v1 on it, and "before" is that commit unless
> noted. No KASAN or lockdep report on the server in any run, nor on the
> clients and the target in tests 2, 3 and 5, where they were checked.
>
> 1. nfsd threads (patch 2), 4 nfsd threads. A client holds a layout of
> a file and its path to the device is cut for 70 s. Another client,
> without pNFS, sends 4 KiB WRITEs to the file through the server, all
> at once, and reads 4 KiB of another file every second:
>
> before v1 v2
> 8 WRITEs, NFSv4.0: the WRITEs take 46.7 s - 45.2 s
> longest read of the other file 46.7 s - 0.031 s
> 64 WRITEs, NFSv4.0: the WRITEs take - 49.2 s 45.4 s
> longest read of the other file - 49.1 s 0.22 s
> 8 WRITEs, NFSv3: the WRITEs take 47.0 s - 51.2 s
> longest read of the other file 46.9 s - 0.021 s
>
> The WRITEs wait for the lease break, which ends with
> /sbin/nfsd-recall-failed (patch 4). NFSv3 WRITEs take longer with
> v2 because the client waits 5 s after NFS3ERR_JUKEBOX. In the run
> with 64 WRITEs before this series, the recall failed after 10 ms, as
> described above, and the WRITEs were done in 0.5 s.
>
> 2. What patch 2 costs. Two clients write 1 MiB blocks to a new file for
> 30 s, one through its layout and one (NFSv4.0) through the server,
> three runs. "+ follow-up" adds the nfsd series that goes on top of
> this one and an NFS client fix that it needs.
>
> through the of those, read through the
> layout back as zeroes server
> before 416/200/186 28/131/111 120/325/323
> v2 174/176/170 0/0/0 184/203/220
> v2 + follow-up 170/175/176 0/0/0 284/223/207
>
> The 30 ms wait is what keeps the writer through the server going:
> two earlier revisions of v2 + follow-up that differ only in it gave
> 79/79/80 blocks through the server without the wait and 238/230/137
> with it.
>
> One client doing both, 1 MiB blocks through its layout and 512 byte
> writes through the server:
>
> through the of those, read writes through
> layout back as zeroes the server
> before 534/537/505 7/4/20 32/21/87
> v2 48/46/4 0/0/0 3600/3262/4917
> v2 + follow-up 193/192/184 0/0/0 1082/1488/1669
>
> With v2 alone, each WRITE of that client through the server recalls
> its layout. About half of its LAYOUTGETs then meet that recall and
> get NFS4ERR_RECALLCONFLICT, as before; the other half are in
> progress when the WRITE breaks the lease, and patch 1 fails them.
> The Linux client waits up to 15 s between LAYOUTGETs that fail with
> NFS4ERR_RECALLCONFLICT. The follow-up hands out the layout of such a
> LAYOUTGET and recalls it at once. That needs the referring call the
> follow-up adds to CB_LAYOUTRECALL: tried in this series, the recall
> overtook the LAYOUTGET reply, and in all three runs 92 to 104 of
> about 260 blocks read back as zeroes.
>
> On 214e388464bf, before and v2 gave the same picture, except that in
> one of three runs of the one-client case with v2 one block read back
> as zeroes, through the recall race described under "Not changed".
>
> The series applies to nfsd-testing c32fef6c476d unchanged, and there
> before and v2 gave the same picture again, also for NFSv3, whose
> replies now come from xdrgen. In test 1 the longest read of the
> other file was 47.8 s before and 0.036 s with v2 over NFSv4.0, and
> 48.3 s and 0.019 s over NFSv3, where the WRITEs took 48.4 s and
> 51.3 s. In this test, with two clients, 48, 94 and 90 of 180, 185
> and 181 blocks through the layout read back as zeroes before, and
> none of 161, 177 and 172 with v2; with one client, 8, 0 and 3
> before and none with v2.
>
> Writes to a file without leases: fio, NFSv4.0, no KASAN or lockdep,
> 10 runs of 30 s, medians without and with an earlier revision of v2
> (the same path for such a file): 1 MiB sequential 65.8 and 67.5
> MiB/s, 4 KiB random 262 and 263 IOPS.
>
> 3. Recall polling (patches 3 to 6). With v2 no recall ended in the
> failure path (0 of 9, 9, 63 and 277 in the arms of patch 5) and no
> block written to a new file was lost (0 of 545, 511, 477, 577 and
> 36). Patch 5 has the runs before.
> On c32fef6c476d, four of those arms lost 4, 2, 18 and 9 blocks
> before, with 46 recall failures logged, and none with v2, with no
> recall failure.
>
> 4. One client writing through a block layout whose path to the device
> is cut while it can still talk to the server, and a 4 KiB write to
> the file on the server ("before" of the 15 s case is 32eb1a60b456).
> The client's write that is in flight stays stuck until the path is
> back.
>
> before v2
> path cut for 15 s: the server write takes 0.05 s 15.6 s
> the recall fails completes
> /sbin/nfsd-recall-failed is run at once no
> path cut for 70 s: the server write takes 0.05 s 47.2 s
> /sbin/nfsd-recall-failed is run at once at 47.1 s
>
> The same 70 s case with a script that expires the client it fences
> through /proc/fs/nfsd/clients/<clid>/ctl: with v1's fence worker,
> which runs the script itself, the script never finished and the
> server write went on only after the path was back, after 93 s. With
> v2 the script was done 36 ms after it started, and the server write
> took 46.1 s. With a lease time of 15 s, the recall reaches its limit
> (patch 3) after 30 s, before the lease break times out, and the
> recall failure path runs the script: there it hangs with v2 as well,
> the script in nfsd4_shutdown_callback() and rpciod in
> call_usermodehelper_exec().
>
> 5. A lease break during a LAYOUTGET (patch 1). With v2 the longest of
> 424 writes on the server took 55 ms, and 108 ms with a delay of
> 100 ms added to LAYOUTGET for the test. Patch 1 has the runs without
> it.
>
> 6. SCSI layout. The target runs a kernel with "nvmet: fix preempt of
> another registrant by the reservation holder" [1], so that the
> preempt removes the registration. With v2, the path of a client cut
> for 15 s: the server write takes 15.2 s and the client is not
> fenced; cut for 70 s: 47.6 s, and the fence worker fences the client
> (registrants 2 -> 1).
>
> pynfs BLOCK1, BLOCK2, BLOCK4 and BLOCK5 pass before and with v2 (BLOCK3
> fails in both with a TypeError in the test). The commit messages of
> patches 6 and 7 have pynfs clients that answer as described there.
>
> Not changed by these patches:
>
> - A layout granted between the check of patch 2 and the break in XFS
> still makes the nfsd thread wait. No new layout lease is granted
> while a break is pending, so that needs a LAYOUTGET for a file that
> had no layout at the check. An asynchronous COPY runs in its own
> kthread and still waits there.
>
> - The recall failure path still runs /sbin/nfsd-recall-failed from
> nfsd4_cb_layout_done(), in rpciod, while the callback is in flight.
> A script that expires the client it fences hangs there, because the
> teardown waits for that callback in nfsd4_shutdown_callback(); test 4
> shows it. Neither path looks at the exit status of the script, and
> the recall failure path does not look at the result of
> ->fence_client(): a fence that fails is taken as done there, and the
> layouts it drops end the retries of a fence worker that a lease break
> has started (patch 3 says so).
>
> - nfsd still polls a client that has answered NFS4_OK with the seqid it
> has used, where RFC 8881 section 12.5.3 has the server increment the
> seqid in each request. Patch 5 only makes nfsd bear the answer
> (patch 6 gives a new seqid only to a client that has not answered
> NFS4_OK), and while it polls, nfsd sends a callback every 10 ms for
> up to two lease periods.
>
> - CB_LAYOUTRECALL carries no referring call, as CB_RECALL does since
> "NFSD: Send referring calls with CB_RECALL". When a recall overtakes
> the reply to the LAYOUTGET that created the layout stateid, the Linux
> client answers NFS4ERR_NOMATCHING_LAYOUT, nfsd drops the layout and
> the client uses it afterwards. nfsd also trusts
> NFS4ERR_NOMATCHING_LAYOUT while it still has layouts on the layout
> stateid. The series on top of this one adds the referring call and
> resends the recall once before it trusts that answer, and with it
> recalls a layout handed out by a LAYOUTGET that a lease break met,
> instead of failing that LAYOUTGET (see test 2).
>
> - Which client makes progress when several clients use one file: the
> client that holds a layout now keeps it while it does I/O through
> it, where before its layout was revoked after 10 ms. Nothing here
> limits how long one client can keep others out.
>
> [1] https://lore.kernel.org/r/20261002065931epcms2p1cb2c16f98cf3b05f8c859ad92b0ad465@epcms2p1
>
> ---
> Changes in v2:
> - New patch 2, which recalls the layouts without waiting before an
> operation that modifies the file and answers NFS4ERR_DELAY, as Chuck
> asked:
> https://lore.kernel.org/r/63ef8d7c-086f-4cc8-84bb-72c1b23bf596@slotpi15m67
> - v1 4/6 is now patch 1, ahead of patch 2, which makes its case common.
> v1 1/6 to 3/6 are patches 3 to 5, v1 5/6 and 6/6 are patches 6 and 7.
> - Sashiko on v1:
> - 2/6, the fence script waiting for itself when it expires the client
> through /proc/fs/nfsd/clients/<clid>/ctl: patch 4 runs the script
> in a work item on a new workqueue, nfsd4_fence, and the fence
> worker checks every 100 ms whether it is done. Teardown waits only
> for the fence worker, which no longer waits for the script.
> - 2/6, sleeping fence workers filling system_dfl_wq: the same change;
> a script that runs long occupies nfsd4_fence.
> - 3/6, ktime_get() stops during suspend: patch 3 takes the time with
> ktime_get_boottime().
> - 5/6, si_generation written under sc_lock in ->done and read under
> ls_mutex only: patch 6 reads it under sc_lock in
> nfsd4_preprocess_layout_stateid() as well.
> - 1/6, the result of ->fence_client() not looked at on the recall
> failure path, which also ends the retries of the fence worker: not
> changed, said in patch 3. Patch 3 makes the path reachable after two
> lease periods for a client that keeps answering; before, a client
> from v6.8 on reached it after 10 ms.
> - 2/6, the exit status of the script not looked at: not changed.
> Retrying a failed script would block the file for good on a server
> that has no /sbin/nfsd-recall-failed. Said in patch 4.
> - 6/6, NFS4ERR_NOMATCHING_LAYOUT trusted while layouts are left:
> preexisting, not changed here (see the list above).
> - Rebased onto nfsd-testing c32fef6c476d without changes, and tests 1
> to 3 run again there.
> - Test numbers in the cover from v2; the commit messages of patches 1,
> 4 and 5 keep their runs on nfsd-testing 32eb1a60b456 for the side
> without the patch. Commit messages with one space after a period.
> - Link to v1: https://lore.kernel.org/r/20261006-nfsd-layout-recall-v1-0-b59b31caf342@samsung.com
>
> ---
> Daejun Park (7):
> nfsd: fail a LAYOUTGET in progress when the layout lease is broken
> nfsd: answer NFS4ERR_DELAY instead of waiting for a layout recall
> nfsd: make the time limit of a layout recall work
> nfsd: run the fence script when a block layout lease break times out
> nfsd: keep polling a layout recall answered with NFS4ERR_OLD_STATEID
> nfsd: poll a layout recall with a new seqid when the client is past it
> nfsd: do not fence a client for a layout recall with no layouts left
>
> fs/nfsd/nfs4layouts.c | 301 ++++++++++++++++++++++++++++++++++++++++++++++----
> fs/nfsd/nfs4proc.c | 8 ++
> fs/nfsd/pnfs.h | 15 +++
> fs/nfsd/state.h | 8 ++
> fs/nfsd/trace.h | 2 +
> fs/nfsd/vfs.c | 81 +++++++++++++-
> fs/nfsd/vfs.h | 1 +
> 7 files changed, 391 insertions(+), 25 deletions(-)
> ---
> base-commit: c32fef6c476daa5830f600614657baee59c83339
> change-id: 20261006-nfsd-layout-recall-ba868d33371e
>
> Best regards,
I think 5 and 6 should be squashed together, but the series itself
looks fine. Nice work!
Reviewed-by: Jeff Layton <jlayton@kernel.org>
prev parent reply other threads:[~2026-10-14 11:26 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-14 7:29 [PATCH v2 0/7] nfsd: layout recall and lease break fixes Daejun Park via B4 Relay
2026-10-14 7:29 ` [PATCH v2 1/7] nfsd: fail a LAYOUTGET in progress when the layout lease is broken Daejun Park via B4 Relay
2026-10-14 7:29 ` [PATCH v2 2/7] nfsd: answer NFS4ERR_DELAY instead of waiting for a layout recall Daejun Park via B4 Relay
2026-10-14 20:59 ` Chuck Lever
2026-10-14 7:29 ` [PATCH v2 3/7] nfsd: make the time limit of a layout recall work Daejun Park via B4 Relay
2026-10-14 7:30 ` [PATCH v2 4/7] nfsd: run the fence script when a block layout lease break times out Daejun Park via B4 Relay
2026-10-14 21:02 ` Chuck Lever
2026-10-14 7:30 ` [PATCH v2 5/7] nfsd: keep polling a layout recall answered with NFS4ERR_OLD_STATEID Daejun Park via B4 Relay
2026-10-14 11:06 ` Jeff Layton
2026-10-14 11:25 ` Jeff Layton
2026-10-14 7:30 ` [PATCH v2 6/7] nfsd: poll a layout recall with a new seqid when the client is past it Daejun Park via B4 Relay
2026-10-14 7:30 ` [PATCH v2 7/7] nfsd: do not fence a client for a layout recall with no layouts left Daejun Park via B4 Relay
2026-10-14 11:26 ` Jeff Layton [this message]
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=46ba944d8dcb989c60c276813339dcb8564a823f.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=Dai.Ngo@oracle.com \
--cc=anna@kernel.org \
--cc=bcodding@redhat.com \
--cc=cel@kernel.org \
--cc=daejun7.park@samsung.com \
--cc=hch@lst.de \
--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=trondmy@kernel.org \
/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