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

Jeff Layton <[email protected]>
Newsgroups org.kernel.vger.linux-nfs
Message-ID <[email protected]>
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 <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.