From: Christoph Hellwig <hch@infradead.org>
To: Sergey Bashirov <sergeybashirov@gmail.com>
Cc: "J . Bruce Fields" <bfields@fieldses.org>,
Chuck Lever <chuck.lever@oracle.com>,
linux-nfs@vger.kernel.org,
Konstantin Evtushenko <koevtushenko@yandex.com>
Subject: Re: [PATCH] nfsd: Implement large extent array support in pNFS
Date: Wed, 4 Jun 2025 07:54:31 -0700 [thread overview]
Message-ID: <aEBeJ2FoSmLvZlSc@infradead.org> (raw)
In-Reply-To: <20250604130809.52931-1-sergeybashirov@gmail.com>
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.
next prev parent reply other threads:[~2025-06-04 14:54 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-04 13:07 [PATCH] nfsd: Implement large extent array support in pNFS Sergey Bashirov
2025-06-04 14:10 ` Chuck Lever
2025-06-04 14:54 ` Christoph Hellwig [this message]
2025-06-10 0:36 ` Sergey Bashirov
2025-06-10 5:39 ` Christoph Hellwig
2025-06-10 15:24 ` Sergey Bashirov
2025-06-11 6:55 ` Christoph Hellwig
2025-06-11 12:19 ` Sergey Bashirov
2025-06-12 6:33 ` Christoph Hellwig
2025-06-12 8:13 ` Sergey Bashirov
2025-06-11 13:53 ` Chuck Lever
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=aEBeJ2FoSmLvZlSc@infradead.org \
--to=hch@infradead.org \
--cc=bfields@fieldses.org \
--cc=chuck.lever@oracle.com \
--cc=koevtushenko@yandex.com \
--cc=linux-nfs@vger.kernel.org \
--cc=sergeybashirov@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox