Re: [PATCH 0/6] NFS: size the LAYOUTGET reply buffer for wide flexfiles layouts

[Jeff Layton <jlayton@kernel.org>]

On Thu, 2026-08-13 at 16:41 -0400, Benjamin Coddington wrote:
> The flexfiles layout driver caps its LAYOUTGET reply buffer at a single
> page, which limits a striped layout segment to roughly 28 ff_data_server4
> entries -- far below the 4096-stripe decode limit the client otherwise
> advertises.  A server striping wider than that has no way to hand the
> client a layout: it returns NFS4ERR_TOOSMALL, the client falls back to
> I/O through the MDS, and pNFS never engages for those files.
> 
> This series first makes the client behave sanely when a layout does not
> fit the size it advertised in loga_maxcount, and then lets the reply
> buffer grow on demand up to the session's maximum response size.
> 
> Patch 1 is a standalone fix (Cc: stable).  A server's NFS4ERR_TOOSMALL
> is mapped to -ETOOSMALL at decode and then handled nowhere, so the
> client falls back to the MDS for that one request and re-sends a doomed
> LAYOUTGET on every subsequent pageio attempt.  Suspend pNFS via the
> layout fail bit instead, the way NFS4ERR_LAYOUTUNAVAILABLE already does.
> 
> Patches 2-5 make wide layouts work.  loga_maxcount is derived from the
> reply buffer actually allocated rather than a fixed 4096 (block and SCSI
> layouts were advertising 4KB against a session-sized buffer, so a server
> whose extent list encodes larger than a page got a needless TOOSMALL).
> A TOOSMALL LAYOUTGET is then retried once with the buffer raised to the
> session's maximum response size -- the same bound GETDEVICEINFO already
> uses -- and a non-conformant server that ignores loga_maxcount and
> overruns the buffer outright takes that same recovery path instead of
> today's -EINVAL.  Finally the escalated size is remembered per-server, so
> subsequent opens skip the attempt that is known to fail; this is also
> what lets the LAYOUTGET attached to OPEN succeed against a wide-striping
> server, since that path is best-effort and has no retry of its own.
> 
> The common path is unchanged throughout: the first LAYOUTGET for a
> layout still goes out with the layout driver's default reply buffer, and
> larger buffers are only ever allocated against servers that actually
> hand out wide layouts.
> 
> Patch 6 moves the decoded per-mirror stripe array to kvzalloc_objs(), so
> that a wide stripe array does not depend on a high-order allocation
> succeeding.
> 
> Wire-validated against reffs at stripe widths 2 and 64, exercising both
> the conformant NFS4ERR_TOOSMALL path and the buffer-overrun path.
> 
> Two known gaps are deliberately left for follow-up work:
> 
>   - The decoded per-stripe footprint is heavy: nfs4_ff_layout_ds_stripe
>     is roughly 300 bytes and embeds localio and layoutstats state that
>     most stripes never use.  Both the structure and its array container
>     want rework before very large stripe counts are comfortable.
> 
>   - The client decodes only the first logr_layout entry of a LAYOUTGET
>     reply and silently discards the rest, so it can accept less coverage
>     than it asked for in loga_minlength, and cannot amortize a
>     multi-segment reply.
> 
> A related series, "NFS: flexfiles device notifications and caching for
> wide striped layouts", handles CB_NOTIFY_DEVICEID and scales the device
> caches for the layouts this one makes fetchable.  The two are
> independent and apply cleanly in either order.
> 
> Benjamin Coddington (6):
>   NFSv4.1/pnfs: suspend pNFS on NFS4ERR_TOOSMALL from LAYOUTGET
>   NFSv4.1/pnfs: derive loga_maxcount from the LAYOUTGET reply buffer
>   NFSv4.1/pnfs: retry LAYOUTGET with a larger reply buffer on
>     NFS4ERR_TOOSMALL
>   NFSv4.1/pnfs: treat an oversized LAYOUTGET reply as -EMSGSIZE
>   NFSv4.1/pnfs: remember when a server needs a larger LAYOUTGET reply
>     buffer
>   NFSv4/flexfiles: allocate the per-mirror stripe array with
>     kvzalloc_objs
> 
>  fs/nfs/flexfilelayout/flexfilelayout.c |  6 +--
>  fs/nfs/nfs4proc.c                      |  9 +++++
>  fs/nfs/nfs4xdr.c                       |  2 +-
>  fs/nfs/pnfs.c                          | 54 +++++++++++++++++++++++---
>  include/linux/nfs_fs_sb.h              |  4 ++
>  5 files changed, 65 insertions(+), 10 deletions(-)
> 
> base-commit: db2ddb87143519e20a95aa36c60b36107b736a58

Aside from my minor comment nit, this looks good. 

Reviewed-by: Jeff Layton <jlayton@kernel.org>