All of lore.kernel.org
 help / color / mirror / Atom feed
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>

  parent reply	other threads:[~2026-10-14 11:26 UTC|newest]

Thread overview: 23+ 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 ` Daejun Park
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   ` Daejun Park
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  7:29   ` Daejun Park
2026-10-14 20:59   ` Chuck Lever
2026-10-15  1:03     ` Daejun Park
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:29   ` Daejun Park
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  7:30   ` Daejun Park
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  7:30   ` Daejun Park
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   ` Daejun Park
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  7:30   ` Daejun Park
2026-10-14 11:26 ` Jeff Layton [this message]
2026-10-15  1:03   ` RE:(2) [PATCH v2 0/7] nfsd: layout recall and lease break fixes Daejun Park

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.