Re: [PATCH v3 3/3] NFSD: Implement NFSD_IO_DIRECT for NFS READ

[Chuck Lever <cel@kernel.org>]

On 9/24/25 12:30 AM, NeilBrown wrote:
> nfsd4_encode_readv() calls xdr_reserve_space_vec() passing maxcount from
> the READ request.  The maxcount is passed to xdr_reserve_space()
> repeatedly (one page at a time) where it is added to xdr->buf->page_len
> (where xdr is ->rq_res_stream and xdr->buf is rq_res).
> 
> So the result is often that rq_res.page_len will be maxcount.
> 
> Then nfsd4_encode_readv() calls nfsd_iter_read() which, with this patch,
> will test rq_res.page_len, which will always be non-zero.
> So that isn't what we want.
> 
> (after the read, nfsd4_encode_readv() will call xdr_truncate_encode()
>  which will reduce ->page_len based on how much was read).
> 
> Then nfsd4_encode_readv() calls nfsd_iter_read().
> 
> I don't think we can use ->page_len to determine if it is safe to use
> nfsd_direct_read().

Agreed.


> The conditions where nfsd_direct_read() is safe are
> similar to the conditions where nfsd_splice_read() is safe.  Maybe we
> should use similar code.

That seems to be this bit of logic at the tail of
nfsd4_decode_compound() ?

	argp->splice_ok = nfsd_read_splice_ok(argp->rqstp);
	if (readcount > 1 || max_reply > PAGE_SIZE - auth_slack)
		argp->splice_ok = false;

But it is rather specific to NFSv4 COMPOUND processing.

There is this familiar-looking check in nfsd4_encode_splice_read() :

	/*
	 * Splice read doesn't work if encoding has already wandered
	 * into the XDR buf's page array.
	 */
	if (unlikely(xdr->buf->page_len)) {
		WARN_ON_ONCE(1);
		return nfserr_serverfault;
	}

Which probably works fine without the xdr_reserve_space_vec() call,
and might even be unnecessary.

I think nfsd4_encode_readv() needs to perform the check somehow.
The check for NFSv3 is always going to be simpler.


-- 
Chuck Lever