Re: [PATCH v2 4/4] NFSD: Implement NFSD_IO_DIRECT for NFS READ

[Chuck Lever <cel@kernel.org>]

On 9/17/25 4:29 PM, NeilBrown wrote:
>> +/*
>> + * The byte range of the client's READ request is expanded on both
>> + * ends until it meets the underlying file system's direct I/O
>> + * alignment requirements. After the internal read is complete, the
>> + * byte range of the NFS READ payload is reduced to the byte range
>> + * that was originally requested.
>> + *
>> + * Note that a direct read can be done only when the xdr_buf
>> + * containing the NFS READ reply does not already have contents in
>> + * its .pages array. This is due to potentially restrictive
>> + * alignment requirements on the read buffer. When .page_len and
>> + * @base are zero, the .pages array is guaranteed to be page-
>> + * aligned.
> This para is confusing.
> It starts talking about the xdr_buf not having any contents.  Then it
> transitions to a guarantee of page alignment.
> 
> If the start of the read requests isn't sufficiently aligned then a gap
> will be created in the xdr_buf and that can only be handled at the start
> (using page_base).
> 
> So as you say we need page_len to be zero.  But nowhere in the code is
> this condition tested.

Despite what the comment claims, I had thought that things would work if
the payload started at a page boundary in xdr_buf.pages. But I can see
that page_offset applies only to the first entry in xdr_buf.pages.

So xdr_buf.page_len does need to be zero. That check can be added in
nfsd_iter_read().

I prefer this approach over more elaborate checking against the
dio_mem_alignment parameter because for the overwhelmingly common cases
of both NFSv3 READ and NFSv4 COMPOUND with one READ operation, page_len
will be zero. The extra complication is hard to unit-test and will
almost never be used.


> The closest is "!base" before the call to nfsd_direct_read() but when
> called from nfsd4_encode_readv()
> 
>    base = xdr->buf->page_len & ~PAGE_MASK;
> 
> so ->page_len could be non-zero despite base being zero.


-- 
Chuck Lever