On Wed, Jun 04, 2025 at 04:07:08PM +0300, Sergey Bashirov wrote:
> When pNFS client in block layout mode sends layoutcommit RPC to MDS,
> a variable length array of modified extents is supplied within request.
> This patch allows NFS server to accept such extent arrays if they do not
> fit within single memory page.
How did you manage to trigger such a workload?
Also you patch doesn't apply to current mainline.
> struct iomap *iomaps;
> int nr_iomaps;
>
> - nr_iomaps = nfsd4_block_decode_layoutupdate(lcp->lc_up_layout,
> - lcp->lc_up_len, &iomaps, i_blocksize(inode));
> + nr_iomaps = nfsd4_block_decode_layoutupdate(&lcp->lc_up_layout,
> + lcp->lc_up_len,
> + &iomaps,
> + i_blocksize(inode));
Please drop all the spurious reformatting to a harder to read style.
> +++ b/fs/nfsd/blocklayoutxdr.c
> @@ -103,11 +103,13 @@ nfsd4_block_encode_getdeviceinfo(struct xdr_stream *xdr,
> }
>
> int
> -nfsd4_block_decode_layoutupdate(__be32 *p, u32 len, struct iomap **iomapp,
> - u32 block_size)
> +nfsd4_block_decode_layoutupdate(struct xdr_buf *buf, u32 len,
> + struct iomap **iomapp, u32 block_size)
> {
So if you pass the xdr_buf that already has the length, can't we drop
the len argument?
> + struct xdr_stream xdr;
Also I'm not the expert on the xdr_stream API, but instead of setting
up a sub-buffer here shouldn't we reuse that from the actual XDR
decoding stage, and if that can't work (Chuck is probably a better
source for know how on that then me), at least initialize it in the
main nfsd layoutcommit handler rather than in the layout drivers.
me we'd probably want that initialized in the core nfsd code and not
the layout driver.
> + if (xdr_stream_decode_opaque_fixed(&xdr, &bex.vol_id, sizeof(bex.vol_id)) <
Overly long line.
> + if (!xdr_stream_subsegment(argp->xdr, &lcp->lc_up_layout, lcp->lc_up_len))
Same.
