Re: [RFC PATCH v2 8/8] NFSD: issue READs using O_DIRECT even if IO is misaligned

[Christoph Hellwig <hch@infradead.org>]

On Tue, Jul 08, 2025 at 12:06:19PM -0400, Mike Snitzer wrote:
> +static bool nfsd_analyze_read_dio(struct svc_rqst *rqstp, struct svc_fh *fhp,
> +				  loff_t offset, __u32 len,
> +				  const u32 dio_blocksize,
> +				  loff_t *start, loff_t *end,
> +				  loff_t *start_extra, loff_t *end_extra)

With this amount of arguments, especially out arguments that return
values the better choice is usually to have a struct.

If not using two-tab ndents at least make it a lot more readable..

> +{
> +	ssize_t host_err = bytes_read;
> +	loff_t v;
> +
> +	/* Must remove first start_extra_page from rqstp->rq_bvec */
> +	if (start_extra_page) {
> +		__free_page(start_extra_page);

I can't really follow the logic here.  Why must it be removed (and
freed)?  Can you write a more detailed comment here as the logic
isn't very obvious.

> +		*rq_bvec_numpages -= 1;
> +		v = *rq_bvec_numpages;
> +		for (int i = 0; i < v; i++) {
> +			struct bio_vec *bv = &rqstp->rq_bvec[i+1];
> +			bvec_set_page(&rqstp->rq_bvec[i], bv->bv_page,
> +				      bv->bv_offset, bv->bv_len);
> +		}

This is basically shifting down the bvecs, right?  Why not simply
use memmove?