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

[Christoph Hellwig <hch@infradead.org>]

> +/*
> + * 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.
> + */

Any reason you are not using the full 80 characters available for the
comment?

> +	v = 0;
> +	total = dio_end - dio_start;
> +	while (total && v < rqstp->rq_maxpages &&
> +	       rqstp->rq_next_page < rqstp->rq_page_end) {
> +		len = min_t(size_t, total, PAGE_SIZE);
> +		bvec_set_page(&rqstp->rq_bvec[v], *rqstp->rq_next_page,
> +			      len, 0);
> +
> +		total -= len;
> +		++rqstp->rq_next_page;
> +		++v;

I find this loop style very hard to follow.  Why not do something like:

Why not something like:

	for (v = 0; v < rqstp->rq_maxpages; v++) {
		size_t len = min_t(size_t, total, PAGE_SIZE);

		bvec_set_page(&rqstp->rq_bvec[v], *rqstp->rq_next_page, len, 0);
		rqstp->rq_next_page++;
		total -= len;
		if (!total)
			break;
	}

?

> +	case NFSD_IO_DIRECT:
> +		/* When dio_read_offset_align is zero, dio is not supported */
> +		if (nf->nf_dio_read_offset_align &&
> +		    !rqstp->rq_res.page_len)

Nit: the entire if clause fits into a single line.