On 8/6/25 11:57 AM, Mike Snitzer wrote:
> On Wed, Aug 06, 2025 at 09:18:51AM -0400, Chuck Lever wrote:
>> On 8/5/25 2:44 PM, Mike Snitzer wrote:
>>> From: Mike Snitzer <snitzer@xxxxxxxxxxxxxxx>
>>>
>>> Check the bvec is DIO-aligned while creating it, saves CPU cycles by
>>> avoiding iterating the bvec elements a second time using
>>> iov_iter_is_aligned().
>>>
>>> This prepares for Keith Busch's near-term removal of the
>>> iov_iter_is_aligned() interface. This fixes cel/nfsd-testing commit
>>> 5d78ac1e674b4 ("NFSD: issue READs using O_DIRECT even if IO is
>>> misaligned") and it should be folded into that commit so that NFSD
>>> doesn't require iov_iter_is_aligned() while it is being removed
>>> upstream in parallel.
>>>
>>> Fixes: cel/nfsd-testing 5d78ac1e674b4 ("NFSD: issue READs using O_DIRECT even if IO is misaligned")
>>> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxxxxxxx>
>>> ---
>>> fs/nfsd/vfs.c | 29 +++++++++++++++--------------
>>> 1 file changed, 15 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
>>> index 46189020172fb..e1751d3715264 100644
>>> --- a/fs/nfsd/vfs.c
>>> +++ b/fs/nfsd/vfs.c
>>> @@ -1226,7 +1226,10 @@ __be32 nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
>>> */
>>> offset = read_dio.start;
>>> in_count = read_dio.end - offset;
>>> - kiocb.ki_flags = IOCB_DIRECT;
>>> + /* Verify ondisk DIO alignment, memory addrs checked below */
>>> + if (likely(((offset | in_count) &
>>> + (nf->nf_dio_read_offset_align - 1)) == 0))
>>> + kiocb.ki_flags = IOCB_DIRECT;
>>> }
>>> } else if (nfsd_io_cache_read == NFSD_IO_DONTCACHE)
>>> kiocb.ki_flags = IOCB_DONTCACHE;
>>> @@ -1236,16 +1239,24 @@ __be32 nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
>>> v = 0;
>>> total = in_count;
>>> if (read_dio.start_extra) {
>>> - bvec_set_page(&rqstp->rq_bvec[v++], read_dio.start_extra_page,
>>> + bvec_set_page(&rqstp->rq_bvec[v], read_dio.start_extra_page,
>>> read_dio.start_extra, PAGE_SIZE - read_dio.start_extra);
>>> + if (unlikely((kiocb.ki_flags & IOCB_DIRECT) &&
>>> + rqstp->rq_bvec[v].bv_offset & (nf->nf_dio_mem_align - 1)))
>>> + kiocb.ki_flags &= ~IOCB_DIRECT;
>>> total -= read_dio.start_extra;
>>> + v++;
>>> }
>>> while (total) {
>>> len = min_t(size_t, total, PAGE_SIZE - base);
>>> - bvec_set_page(&rqstp->rq_bvec[v++], *(rqstp->rq_next_page++),
>>> - len, base);
>>> + bvec_set_page(&rqstp->rq_bvec[v], *(rqstp->rq_next_page++), len, base);
>>> + /* No need to verify memory is DIO-aligned since bv_offset is 0 */
>>> + if (unlikely((kiocb.ki_flags & IOCB_DIRECT) && base &&
>>> + (base & (nf->nf_dio_mem_align - 1))))
>>> + kiocb.ki_flags &= ~IOCB_DIRECT;
>>> total -= len;
>>> base = 0;
>>> + v++;
>>> }
>>> if (WARN_ONCE(v > rqstp->rq_maxpages,
>>> "%s: v=%lu exceeds rqstp->rq_maxpages=%lu\n", __func__,
>>> @@ -1256,16 +1267,6 @@ __be32 nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
>>> if (!host_err) {
>>> trace_nfsd_read_vector(rqstp, fhp, offset, in_count);
>>> iov_iter_bvec(&iter, ITER_DEST, rqstp->rq_bvec, v, in_count);
>>> -
>>> - /* Double check nfsd_analyze_read_dio's DIO-aligned result */
>>> - if (unlikely((kiocb.ki_flags & IOCB_DIRECT) &&
>>> - !iov_iter_is_aligned(&iter,
>>> - nf->nf_dio_mem_align - 1,
>>> - nf->nf_dio_read_offset_align - 1))) {
>>> - /* Fallback to buffered IO */
>>> - kiocb.ki_flags &= ~IOCB_DIRECT;
>>> - }
>>> -
>>> host_err = vfs_iocb_iter_read(file, &kiocb, &iter);
>>> }
>>>
>>
>> Hi Mike,
>>
>> In cases where the SQUASHME patch is this large, I usually drop the
>> patch (or series) in nfsd-testing and ask the contributor to rebase and
>> repost. This gets the new version of the patch properly archived on
>> lore, for one thing.
>
> Yeah, make sense, I missed that iov_iter_is_aligned() was used early
> on in the series too, so I'll fixup further back.
>
>> Before reposting, please do run checkpatch.pl on the series.
>
> Will do, will also ensure bisect safe and that sparse is happy.
Thanks for the changes, sounds like the right path forward.
--
Chuck Lever
