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]>