Re: [PATCH v3 5/5] nfs: introduce nfs_direct_extract_pages helper

On Wed, 2026-07-15 at 14:35 +0000, Pranjal Shrivastava wrote:
> Introduce nfs_direct_extract_pages() in direct.c to centralize page
> extraction and request creation for the Direct I/O path. The helper
> manages extraction from the iters and builds a list of nfs_page
> requests
>=20
> Refactor nfs_direct_read_schedule_iovec() and
> nfs_direct_write_schedule_iovec() to utilize the new helper, unifying
> the extraction logic on both paths.
>=20
> Signed-off-by: Pranjal Shrivastava <[email protected]>
> ---
> =C2=A0fs/nfs/direct.c | 127 ++++++++++++++++++++++++---------------------=
-
> --
> =C2=A01 file changed, 64 insertions(+), 63 deletions(-)
>=20
> diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
> index b9ac0a67693c..d31e4720ffff 100644
> --- a/fs/nfs/direct.c
> +++ b/fs/nfs/direct.c
> @@ -178,6 +178,50 @@ static void nfs_direct_release_pages(struct page
> **pages, unsigned int npages,
> =C2=A0	}
> =C2=A0}
> =C2=A0
> +static ssize_t nfs_direct_extract_pages(struct nfs_direct_req *dreq,
> +					 struct iov_iter *iter,
> +					 size_t size, loff_t *pos,
> +					 struct list_head *list)
> +{
> +	bool pinned =3D iov_iter_extract_will_pin(iter);
> +	struct page **pagevec =3D NULL;
> +	ssize_t result, bytes =3D 0;
> +	unsigned int npages, i;
> +	size_t pgbase;
> +
> +	result =3D iov_iter_extract_pages(iter, &pagevec, size, ~0U,
> 0, &pgbase);
> +	if (result <=3D 0)
> +		return result;
> +
> +	npages =3D (result + pgbase + PAGE_SIZE - 1) >> PAGE_SHIFT;
> +	for (i =3D 0; i < npages; i++) {
> +		struct nfs_page *req;
> +		unsigned int req_len =3D min_t(size_t, result - bytes,
> PAGE_SIZE - pgbase);
> +
> +		req =3D nfs_page_create_from_page(dreq->ctx,
> pagevec[i],
> +						pinned, pgbase,
> *pos,
> +						req_len);
> +		if (IS_ERR(req)) {
> +			if (!bytes)
> +				bytes =3D PTR_ERR(req);
> +			break;
> +		}
> +
> +		list_add_tail(&req->wb_list, list);
> +		pgbase =3D 0;
> +		bytes +=3D req_len;
> +		*pos +=3D req_len;
> +	}
> +
> +	if (i < npages) {
> +		iov_iter_revert(iter, result - bytes);

Oopsable if bytes =3D=3D PTR_ERR(req)

> +		nfs_direct_release_pages(pagevec + i, npages - i,
> pinned);

See what happens above with swap over NFS (which sets pinned =3D false).

> +	}
> +
> +	kvfree(pagevec);
> +	return bytes;
> +}
> +
> =C2=A0void nfs_init_cinfo_from_dreq(struct nfs_commit_info *cinfo,
> =C2=A0			=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct nfs_direct_req *dreq)
> =C2=A0{
> @@ -346,6 +390,7 @@ static ssize_t
> nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
> =C2=A0	ssize_t result =3D -EINVAL;
> =C2=A0	size_t requested_bytes =3D 0;
> =C2=A0	size_t rsize =3D max_t(size_t, NFS_SERVER(inode)->rsize,
> PAGE_SIZE);
> +	LIST_HEAD(nfs_page_list);
> =C2=A0
> =C2=A0	nfs_pageio_init_read(&desc, dreq->inode, false,
> =C2=A0			=C2=A0=C2=A0=C2=A0=C2=A0 &nfs_direct_read_completion_ops);
> @@ -354,43 +399,23 @@ static ssize_t
> nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
> =C2=A0	inode_dio_begin(inode);
> =C2=A0
> =C2=A0	while (iov_iter_count(iter)) {
> -		struct page **pagevec =3D NULL;
> -		size_t bytes;
> -		size_t pgbase;
> -		unsigned npages, i;
> -		bool pinned =3D iov_iter_extract_will_pin(iter);
> -
> -		result =3D iov_iter_extract_pages(iter, &pagevec,
> -						rsize, ~0U, 0,
> &pgbase);
> +		result =3D nfs_direct_extract_pages(dreq, iter, rsize,
> &pos, &nfs_page_list);
> =C2=A0		if (result < 0)
> =C2=A0			break;
> =C2=A0
> -		bytes =3D result;
> -		npages =3D (result + pgbase + PAGE_SIZE - 1) /
> PAGE_SIZE;
> -		for (i =3D 0; i < npages; i++) {
> -			struct nfs_page *req;
> -			unsigned int req_len =3D min_t(size_t, bytes,
> PAGE_SIZE - pgbase);
> -			/* XXX do we need to do the eof zeroing
> found in async_filler? */
> -			req =3D nfs_page_create_from_page(dreq->ctx,
> pagevec[i],
> -							pinned,
> pgbase, pos,
> -							req_len);
> -			if (IS_ERR(req)) {
> -				result =3D PTR_ERR(req);
> -				break;
> -			}
> +		while (!list_empty(&nfs_page_list)) {
> +			struct nfs_page *req =3D
> nfs_list_entry(nfs_page_list.next);
> +			size_t req_len =3D req->wb_bytes;
> +
> +			nfs_list_remove_request(req);
> =C2=A0			if (!nfs_pageio_add_request(&desc, req)) {
> =C2=A0				result =3D desc.pg_error;
> =C2=A0				nfs_release_request(req);
> +				nfs_release_request_list(&nfs_page_l
> ist);
> =C2=A0				break;
> =C2=A0			}
> -			pgbase =3D 0;
> -			bytes -=3D req_len;
> =C2=A0			requested_bytes +=3D req_len;
> -			pos +=3D req_len;
> =C2=A0		}
> -		if (i < npages)
> -			nfs_direct_release_pages(pagevec + i, npages
> - i, pinned);
> -		kvfree(pagevec);
> =C2=A0		if (result < 0)
> =C2=A0			break;
> =C2=A0	}
> @@ -881,6 +906,7 @@ static ssize_t
> nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
> =C2=A0	ssize_t result =3D 0;
> =C2=A0	size_t requested_bytes =3D 0;
> =C2=A0	size_t wsize =3D max_t(size_t, NFS_SERVER(inode)->wsize,
> PAGE_SIZE);
> +	LIST_HEAD(nfs_page_list);
> =C2=A0	bool defer =3D false;
> =C2=A0
> =C2=A0	trace_nfs_direct_write_schedule_iovec(dreq);
> @@ -893,55 +919,32 @@ static ssize_t
> nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
> =C2=A0
> =C2=A0	NFS_I(inode)->write_io +=3D iov_iter_count(iter);
> =C2=A0	while (iov_iter_count(iter)) {
> -		struct page **pagevec =3D NULL;
> -		size_t bytes;
> -		size_t pgbase;
> -		unsigned npages, i;
> -		bool pinned =3D iov_iter_extract_will_pin(iter);
> -
> -		result =3D iov_iter_extract_pages(iter, &pagevec,
> -						wsize, ~0U, 0,
> &pgbase);
> +		result =3D nfs_direct_extract_pages(dreq, iter, wsize,
> &pos, &nfs_page_list);
> =C2=A0		if (result < 0)
> =C2=A0			break;
> =C2=A0
> -		bytes =3D result;
> -		npages =3D (result + pgbase + PAGE_SIZE - 1) /
> PAGE_SIZE;
> -		for (i =3D 0; i < npages; i++) {
> -			struct nfs_page *req;
> -			unsigned int req_len =3D min_t(size_t, bytes,
> PAGE_SIZE - pgbase);
> -
> -			req =3D nfs_page_create_from_page(dreq->ctx,
> pagevec[i],
> -							pinned,
> pgbase, pos,
> -							req_len);
> -			if (IS_ERR(req)) {
> -				result =3D PTR_ERR(req);
> -				break;
> -			}
> -
> -			if (desc.pg_error < 0) {
> -				nfs_free_request(req);
> -				result =3D desc.pg_error;
> -				break;
> -			}
> -
> -			pgbase =3D 0;
> -			bytes -=3D req_len;
> -			requested_bytes +=3D req_len;
> -			pos +=3D req_len;
> +		while (!list_empty(&nfs_page_list)) {
> +			struct nfs_page *req =3D
> nfs_list_entry(nfs_page_list.next);
> +			size_t req_len =3D req->wb_bytes;
> =C2=A0
> +			nfs_list_remove_request(req);
> =C2=A0			if (defer) {
> =C2=A0				nfs_mark_request_commit(req, NULL,
> &cinfo, 0);
> +				requested_bytes +=3D req_len;
> =C2=A0				continue;
> =C2=A0			}
> =C2=A0
> =C2=A0			nfs_lock_request(req);
> -			if (nfs_pageio_add_request(&desc, req))
> +			if (nfs_pageio_add_request(&desc, req)) {
> +				requested_bytes +=3D req_len;
> =C2=A0				continue;
> +			}
> =C2=A0
> =C2=A0			/* Exit on hard errors */
> =C2=A0			if (desc.pg_error < 0 && desc.pg_error !=3D -
> EAGAIN) {
> =C2=A0				result =3D desc.pg_error;
> =C2=A0				nfs_unlock_and_release_request(req);
> +				nfs_release_request_list(&nfs_page_l
> ist);
> =C2=A0				break;
> =C2=A0			}
> =C2=A0
> @@ -952,12 +955,10 @@ static ssize_t
> nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
> =C2=A0			spin_unlock(&dreq->lock);
> =C2=A0			nfs_unlock_request(req);
> =C2=A0			nfs_mark_request_commit(req, NULL, &cinfo,
> 0);
> +			requested_bytes +=3D req_len;
> =C2=A0			desc.pg_error =3D 0;
> =C2=A0			defer =3D true;
> =C2=A0		}
> -		if (i < npages)
> -			nfs_direct_release_pages(pagevec + i, npages
> - i, pinned);
> -		kvfree(pagevec);
> =C2=A0		if (result < 0)
> =C2=A0			break;
> =C2=A0	}

--=20
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected], trond.myklebust-F/[email protected]