* [PATCH v3 1/5] nfs: make nfs_page pin-aware
2026-07-15 14:35 [PATCH v3 0/5] nfs: Modernize Direct I/O path Pranjal Shrivastava
@ 2026-07-15 14:35 ` Pranjal Shrivastava
2026-07-20 9:13 ` Christoph Hellwig
2026-07-15 14:35 ` [PATCH v3 2/5] nfs: Track number of pinned pages in nfs_page Pranjal Shrivastava
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Pranjal Shrivastava @ 2026-07-15 14:35 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, linux-nfs
Cc: Chuck Lever, Jeff Layton, linux-kernel, Christoph Hellwig,
Logan Gunthorpe, Jason Gunthorpe, linux-pci, linux-rdma,
Shivaji Kant, Pranjal Shrivastava
Modernizing the NFS Direct I/O path to use iov_iter_extract_pages()
introduces page pinning (GUP) instead of standard page referencing.
To handle this correctly, nfs_page must track whether it holds a
pin or a standard reference.
Introduce a new flag, PG_PINNED, to struct nfs_page. Update the creation
path (nfs_page_create_from_page and nfs_page_create_from_folio) to
accept a pinned bool and set the flag accordingly. If the page is pinned,
we skip the existing reference increment (get_page/folio_get) as the pin
itself acts as a reference.
Update nfs_clear_request() & nfs_direct_release_pages() to use
unpin_user_page() or unpin_user_folio() instead of only refcount
decrement (put_page) when PG_PINNED flag is set. Finally, ensure
subrequests inherit the pinning status from their parent request.
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
fs/nfs/direct.c | 22 +++++++++++++++-------
fs/nfs/pagelist.c | 38 ++++++++++++++++++++++++++++----------
fs/nfs/read.c | 2 +-
fs/nfs/write.c | 2 +-
include/linux/nfs_page.h | 3 +++
5 files changed, 48 insertions(+), 19 deletions(-)
diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index e626c72495e6..19792a38c924 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -165,11 +165,17 @@ int nfs_swap_rw(struct kiocb *iocb, struct iov_iter *iter)
return 0;
}
-static void nfs_direct_release_pages(struct page **pages, unsigned int npages)
+static void nfs_direct_release_pages(struct page **pages, unsigned int npages,
+ bool pinned)
{
unsigned int i;
- for (i = 0; i < npages; i++)
- put_page(pages[i]);
+
+ if (pinned) {
+ unpin_user_pages(pages, npages);
+ } else {
+ for (i = 0; i < npages; i++)
+ put_page(pages[i]);
+ }
}
void nfs_init_cinfo_from_dreq(struct nfs_commit_info *cinfo,
@@ -371,7 +377,8 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
unsigned int req_len = min_t(size_t, bytes, PAGE_SIZE - pgbase);
/* XXX do we need to do the eof zeroing found in async_filler? */
req = nfs_page_create_from_page(dreq->ctx, pagevec[i],
- pgbase, pos, req_len);
+ false, pgbase, pos,
+ req_len);
if (IS_ERR(req)) {
result = PTR_ERR(req);
break;
@@ -386,7 +393,7 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
requested_bytes += req_len;
pos += req_len;
}
- nfs_direct_release_pages(pagevec, npages);
+ nfs_direct_release_pages(pagevec, npages, false);
kvfree(pagevec);
if (result < 0)
break;
@@ -907,7 +914,8 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
unsigned int req_len = min_t(size_t, bytes, PAGE_SIZE - pgbase);
req = nfs_page_create_from_page(dreq->ctx, pagevec[i],
- pgbase, pos, req_len);
+ false, pgbase, pos,
+ req_len);
if (IS_ERR(req)) {
result = PTR_ERR(req);
break;
@@ -950,7 +958,7 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
desc.pg_error = 0;
defer = true;
}
- nfs_direct_release_pages(pagevec, npages);
+ nfs_direct_release_pages(pagevec, npages, false);
kvfree(pagevec);
if (result < 0)
break;
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 7dd478ffc2fa..faa8bc1c6526 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -404,20 +404,26 @@ static struct nfs_page *nfs_page_create(struct nfs_lock_context *l_ctx,
return req;
}
-static void nfs_page_assign_folio(struct nfs_page *req, struct folio *folio)
+static void nfs_page_assign_folio(struct nfs_page *req, struct folio *folio, bool pinned)
{
if (folio != NULL) {
req->wb_folio = folio;
- folio_get(folio);
+ if (pinned)
+ set_bit(PG_PINNED, &req->wb_flags);
+ else
+ folio_get(folio);
set_bit(PG_FOLIO, &req->wb_flags);
}
}
-static void nfs_page_assign_page(struct nfs_page *req, struct page *page)
+static void nfs_page_assign_page(struct nfs_page *req, struct page *page, bool pinned)
{
if (page != NULL) {
req->wb_page = page;
- get_page(page);
+ if (pinned)
+ set_bit(PG_PINNED, &req->wb_flags);
+ else
+ get_page(page);
}
}
@@ -425,6 +431,7 @@ static void nfs_page_assign_page(struct nfs_page *req, struct page *page)
* nfs_page_create_from_page - Create an NFS read/write request.
* @ctx: open context to use
* @page: page to write
+ * @pinned: true if page is pinned
* @pgbase: starting offset within the page for the write
* @offset: file offset for the write
* @count: number of bytes to read/write
@@ -435,6 +442,7 @@ static void nfs_page_assign_page(struct nfs_page *req, struct page *page)
*/
struct nfs_page *nfs_page_create_from_page(struct nfs_open_context *ctx,
struct page *page,
+ bool pinned,
unsigned int pgbase, loff_t offset,
unsigned int count)
{
@@ -446,7 +454,7 @@ struct nfs_page *nfs_page_create_from_page(struct nfs_open_context *ctx,
ret = nfs_page_create(l_ctx, pgbase, offset >> PAGE_SHIFT,
offset_in_page(offset), count);
if (!IS_ERR(ret)) {
- nfs_page_assign_page(ret, page);
+ nfs_page_assign_page(ret, page, pinned);
nfs_page_group_init(ret, NULL);
}
nfs_put_lock_context(l_ctx);
@@ -457,6 +465,7 @@ struct nfs_page *nfs_page_create_from_page(struct nfs_open_context *ctx,
* nfs_page_create_from_folio - Create an NFS read/write request.
* @ctx: open context to use
* @folio: folio to write
+ * @pinned: true if folio is pinned
* @offset: starting offset within the folio for the write
* @count: number of bytes to read/write
*
@@ -466,6 +475,7 @@ struct nfs_page *nfs_page_create_from_page(struct nfs_open_context *ctx,
*/
struct nfs_page *nfs_page_create_from_folio(struct nfs_open_context *ctx,
struct folio *folio,
+ bool pinned,
unsigned int offset,
unsigned int count)
{
@@ -476,7 +486,7 @@ struct nfs_page *nfs_page_create_from_folio(struct nfs_open_context *ctx,
return ERR_CAST(l_ctx);
ret = nfs_page_create(l_ctx, offset, folio->index, offset, count);
if (!IS_ERR(ret)) {
- nfs_page_assign_folio(ret, folio);
+ nfs_page_assign_folio(ret, folio, pinned);
nfs_page_group_init(ret, NULL);
}
nfs_put_lock_context(l_ctx);
@@ -498,9 +508,11 @@ nfs_create_subreq(struct nfs_page *req,
offset, count);
if (!IS_ERR(ret)) {
if (folio)
- nfs_page_assign_folio(ret, folio);
+ nfs_page_assign_folio(ret, folio,
+ test_bit(PG_PINNED, &req->wb_flags));
else
- nfs_page_assign_page(ret, page);
+ nfs_page_assign_page(ret, page,
+ test_bit(PG_PINNED, &req->wb_flags));
/* find the last request */
for (last = req->wb_head;
last->wb_this_page != req->wb_head;
@@ -552,11 +564,17 @@ static void nfs_clear_request(struct nfs_page *req)
struct nfs_open_context *ctx;
if (folio != NULL) {
- folio_put(folio);
+ if (test_and_clear_bit(PG_PINNED, &req->wb_flags))
+ unpin_user_folio(folio, 1);
+ else
+ folio_put(folio);
req->wb_folio = NULL;
clear_bit(PG_FOLIO, &req->wb_flags);
} else if (page != NULL) {
- put_page(page);
+ if (test_and_clear_bit(PG_PINNED, &req->wb_flags))
+ unpin_user_page(page);
+ else
+ put_page(page);
req->wb_page = NULL;
}
if (l_ctx != NULL) {
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 2b70bd2b934b..e7497b029d6c 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -324,7 +324,7 @@ int nfs_read_add_folio(struct nfs_pageio_descriptor *pgio,
aligned_len = min_t(unsigned int, ALIGN(len, rsize), fsize);
- new = nfs_page_create_from_folio(ctx, folio, 0, aligned_len);
+ new = nfs_page_create_from_folio(ctx, folio, false, 0, aligned_len);
if (IS_ERR(new)) {
error = PTR_ERR(new);
if (nfs_netfs_folio_unlock(folio))
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index d2b03ceaeb4f..e565b811d8b7 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1088,7 +1088,7 @@ static struct nfs_page *nfs_setup_write_request(struct nfs_open_context *ctx,
req = nfs_try_to_update_request(folio, offset, bytes);
if (req != NULL)
goto out;
- req = nfs_page_create_from_folio(ctx, folio, offset, bytes);
+ req = nfs_page_create_from_folio(ctx, folio, false, offset, bytes);
if (IS_ERR(req))
goto out;
nfs_inode_add_request(req);
diff --git a/include/linux/nfs_page.h b/include/linux/nfs_page.h
index 4b9a35dbc062..fd7aafe7cb54 100644
--- a/include/linux/nfs_page.h
+++ b/include/linux/nfs_page.h
@@ -38,6 +38,7 @@ enum {
PG_REMOVE, /* page group sync bit in write path */
PG_CONTENDED1, /* Is someone waiting for a lock? */
PG_CONTENDED2, /* Is someone waiting for a lock? */
+ PG_PINNED, /* page is pinned by GUP */
};
struct nfs_inode;
@@ -125,11 +126,13 @@ struct nfs_pageio_descriptor {
extern struct nfs_page *nfs_page_create_from_page(struct nfs_open_context *ctx,
struct page *page,
+ bool pinned,
unsigned int pgbase,
loff_t offset,
unsigned int count);
extern struct nfs_page *nfs_page_create_from_folio(struct nfs_open_context *ctx,
struct folio *folio,
+ bool pinned,
unsigned int offset,
unsigned int count);
extern void nfs_release_request(struct nfs_page *);
--
2.55.0.229.g6434b31f56-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 1/5] nfs: make nfs_page pin-aware
2026-07-15 14:35 ` [PATCH v3 1/5] nfs: make nfs_page pin-aware Pranjal Shrivastava
@ 2026-07-20 9:13 ` Christoph Hellwig
2026-07-20 13:52 ` Pranjal Shrivastava
0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2026-07-20 9:13 UTC (permalink / raw)
To: Pranjal Shrivastava
Cc: Trond Myklebust, Anna Schumaker, linux-nfs, Chuck Lever,
Jeff Layton, linux-kernel, Christoph Hellwig, Logan Gunthorpe,
Jason Gunthorpe, linux-pci, linux-rdma, Shivaji Kant
It would be nice to avoid the overly long lines, but otherwise this
looks good.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/5] nfs: make nfs_page pin-aware
2026-07-20 9:13 ` Christoph Hellwig
@ 2026-07-20 13:52 ` Pranjal Shrivastava
0 siblings, 0 replies; 17+ messages in thread
From: Pranjal Shrivastava @ 2026-07-20 13:52 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Trond Myklebust, Anna Schumaker, linux-nfs, Chuck Lever,
Jeff Layton, linux-kernel, Logan Gunthorpe, Jason Gunthorpe,
linux-pci, linux-rdma, Shivaji Kant
On Mon, Jul 20, 2026 at 11:13:22AM +0200, Christoph Hellwig wrote:
> It would be nice to avoid the overly long lines, but otherwise this
> looks good.
>
Ack, I'll trim the long lines and post a v4 quickly.
Thanks!
Praan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 2/5] nfs: Track number of pinned pages in nfs_page
2026-07-15 14:35 [PATCH v3 0/5] nfs: Modernize Direct I/O path Pranjal Shrivastava
2026-07-15 14:35 ` [PATCH v3 1/5] nfs: make nfs_page pin-aware Pranjal Shrivastava
@ 2026-07-15 14:35 ` Pranjal Shrivastava
2026-07-20 9:14 ` Christoph Hellwig
2026-07-15 14:35 ` [PATCH v3 3/5] nfs: Introduce nfs_release_request_list helper Pranjal Shrivastava
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Pranjal Shrivastava @ 2026-07-15 14:35 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, linux-nfs
Cc: Chuck Lever, Jeff Layton, linux-kernel, Christoph Hellwig,
Logan Gunthorpe, Jason Gunthorpe, linux-pci, linux-rdma,
Shivaji Kant, Pranjal Shrivastava
Track the number of pinned pages in nfs_page to handle unpinning
correctly, ensuring that only primary requests perform the final
unpinning operation, preventing subrequests from incorrectly
performing unpinning on behalf of their parent requests.
Add wb_nr_pinned to struct nfs_page to store the count of pinned pages
owned by the request. Update request creation and cleanup helpers to
initialize and use wb_nr_pinned for primary requests. Use the
nfs_page_array_len() helper to calculate the number of pages spanned
by a request's offset and length.
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
fs/nfs/pagelist.c | 9 +++++++--
include/linux/nfs_page.h | 1 +
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index faa8bc1c6526..7d51e10fe97a 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -455,6 +455,8 @@ struct nfs_page *nfs_page_create_from_page(struct nfs_open_context *ctx,
offset_in_page(offset), count);
if (!IS_ERR(ret)) {
nfs_page_assign_page(ret, page, pinned);
+ if (pinned)
+ ret->wb_nr_pinned = 1;
nfs_page_group_init(ret, NULL);
}
nfs_put_lock_context(l_ctx);
@@ -487,6 +489,9 @@ struct nfs_page *nfs_page_create_from_folio(struct nfs_open_context *ctx,
ret = nfs_page_create(l_ctx, offset, folio->index, offset, count);
if (!IS_ERR(ret)) {
nfs_page_assign_folio(ret, folio, pinned);
+ if (pinned)
+ ret->wb_nr_pinned = nfs_page_array_len(offset_in_page(offset),
+ count);
nfs_page_group_init(ret, NULL);
}
nfs_put_lock_context(l_ctx);
@@ -565,14 +570,14 @@ static void nfs_clear_request(struct nfs_page *req)
if (folio != NULL) {
if (test_and_clear_bit(PG_PINNED, &req->wb_flags))
- unpin_user_folio(folio, 1);
+ unpin_user_folio(folio, req->wb_nr_pinned);
else
folio_put(folio);
req->wb_folio = NULL;
clear_bit(PG_FOLIO, &req->wb_flags);
} else if (page != NULL) {
if (test_and_clear_bit(PG_PINNED, &req->wb_flags))
- unpin_user_page(page);
+ unpin_user_pages(&page, req->wb_nr_pinned);
else
put_page(page);
req->wb_page = NULL;
diff --git a/include/linux/nfs_page.h b/include/linux/nfs_page.h
index fd7aafe7cb54..080fa3e23580 100644
--- a/include/linux/nfs_page.h
+++ b/include/linux/nfs_page.h
@@ -59,6 +59,7 @@ struct nfs_page {
struct nfs_page *wb_this_page; /* list of reqs for this page */
struct nfs_page *wb_head; /* head pointer for req list */
unsigned short wb_nio; /* Number of I/O attempts */
+ unsigned int wb_nr_pinned; /* Number of pinned pages */
};
struct nfs_pgio_mirror;
--
2.55.0.229.g6434b31f56-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 2/5] nfs: Track number of pinned pages in nfs_page
2026-07-15 14:35 ` [PATCH v3 2/5] nfs: Track number of pinned pages in nfs_page Pranjal Shrivastava
@ 2026-07-20 9:14 ` Christoph Hellwig
2026-07-20 14:05 ` Pranjal Shrivastava
0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2026-07-20 9:14 UTC (permalink / raw)
To: Pranjal Shrivastava
Cc: Trond Myklebust, Anna Schumaker, linux-nfs, Chuck Lever,
Jeff Layton, linux-kernel, Christoph Hellwig, Logan Gunthorpe,
Jason Gunthorpe, linux-pci, linux-rdma, Shivaji Kant
Nit: capitalization of the patch subjects is inconsistent. You probably
one to stick to one version or another.
Otherwise this looks good to me.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/5] nfs: Track number of pinned pages in nfs_page
2026-07-20 9:14 ` Christoph Hellwig
@ 2026-07-20 14:05 ` Pranjal Shrivastava
0 siblings, 0 replies; 17+ messages in thread
From: Pranjal Shrivastava @ 2026-07-20 14:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Trond Myklebust, Anna Schumaker, linux-nfs, Chuck Lever,
Jeff Layton, linux-kernel, Logan Gunthorpe, Jason Gunthorpe,
linux-pci, linux-rdma, Shivaji Kant
On Mon, Jul 20, 2026 at 11:14:01AM +0200, Christoph Hellwig wrote:
> Nit: capitalization of the patch subjects is inconsistent. You probably
> one to stick to one version or another.
Ack. I'll stay with the lower-case convention as it seems to be common
in the other commits. I'll fix that in the v4.
>
> Otherwise this looks good to me.
>
Thanks!
Praan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 3/5] nfs: Introduce nfs_release_request_list helper
2026-07-15 14:35 [PATCH v3 0/5] nfs: Modernize Direct I/O path Pranjal Shrivastava
2026-07-15 14:35 ` [PATCH v3 1/5] nfs: make nfs_page pin-aware Pranjal Shrivastava
2026-07-15 14:35 ` [PATCH v3 2/5] nfs: Track number of pinned pages in nfs_page Pranjal Shrivastava
@ 2026-07-15 14:35 ` Pranjal Shrivastava
2026-07-20 9:15 ` Christoph Hellwig
2026-07-15 14:35 ` [PATCH v3 4/5] nfs: migrate direct I/O to iov_iter_extract_pages Pranjal Shrivastava
2026-07-15 14:35 ` [PATCH v3 5/5] nfs: introduce nfs_direct_extract_pages helper Pranjal Shrivastava
4 siblings, 1 reply; 17+ messages in thread
From: Pranjal Shrivastava @ 2026-07-15 14:35 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, linux-nfs
Cc: Chuck Lever, Jeff Layton, linux-kernel, Christoph Hellwig,
Logan Gunthorpe, Jason Gunthorpe, linux-pci, linux-rdma,
Shivaji Kant, Pranjal Shrivastava
Introduce a centralized helper, nfs_release_request_list, to handle
the bulk release of nfs_page requests from a list.
This serves as a preparatory step for two upcoming improvements:
1. Pin-Aware Cleanup: As we migrate to iov_iter_extract_* API,
requests will hold pins (GUP) instead of standard references. The
helper ensures that the correct unpinning logic gets applied
consistently across all requests in a list.
2. Folio Support: In subsequent patches where nfs_page structures
will cover multi-page folios, this helper provides a clean
infrastructure to unlock these larger units of I/O in bulk during
completion, similat to the pattern in bio_release_pages.
Additionally, refactor nfs_read_sync_pgio_error() to utilize this new
helper.
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
fs/nfs/direct.c | 8 +-------
fs/nfs/pagelist.c | 18 ++++++++++++++++++
include/linux/nfs_page.h | 4 ++--
3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index 19792a38c924..96995736fac2 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -314,13 +314,7 @@ static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)
static void nfs_read_sync_pgio_error(struct list_head *head, int error)
{
- struct nfs_page *req;
-
- while (!list_empty(head)) {
- req = nfs_list_entry(head->next);
- nfs_list_remove_request(req);
- nfs_release_request(req);
- }
+ nfs_release_request_list(head);
}
static void nfs_direct_pgio_init(struct nfs_pgio_header *hdr)
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 7d51e10fe97a..569bac4faff7 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -622,6 +622,24 @@ void nfs_release_request(struct nfs_page *req)
}
EXPORT_SYMBOL_GPL(nfs_release_request);
+/*
+ * nfs_release_request_list - Release a list of NFS read/write requests
+ * @head: list of requests to release
+ *
+ * Removes each request from the list and drops it's refcount.
+ */
+void nfs_release_request_list(struct list_head *head)
+{
+ struct nfs_page *req;
+
+ while (!list_empty(head)) {
+ req = nfs_list_entry(head->next);
+ nfs_list_remove_request(req);
+ nfs_release_request(req);
+ }
+}
+EXPORT_SYMBOL_GPL(nfs_release_request_list);
+
/*
* nfs_generic_pg_test - determine if requests can be coalesced
* @desc: pointer to descriptor
diff --git a/include/linux/nfs_page.h b/include/linux/nfs_page.h
index 080fa3e23580..d23208ed3a33 100644
--- a/include/linux/nfs_page.h
+++ b/include/linux/nfs_page.h
@@ -136,8 +136,8 @@ extern struct nfs_page *nfs_page_create_from_folio(struct nfs_open_context *ctx,
bool pinned,
unsigned int offset,
unsigned int count);
-extern void nfs_release_request(struct nfs_page *);
-
+extern void nfs_release_request(struct nfs_page *req);
+extern void nfs_release_request_list(struct list_head *head);
extern void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
struct inode *inode,
--
2.55.0.229.g6434b31f56-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 3/5] nfs: Introduce nfs_release_request_list helper
2026-07-15 14:35 ` [PATCH v3 3/5] nfs: Introduce nfs_release_request_list helper Pranjal Shrivastava
@ 2026-07-20 9:15 ` Christoph Hellwig
2026-07-20 9:58 ` Shivaji Kant
2026-07-20 14:24 ` Pranjal Shrivastava
0 siblings, 2 replies; 17+ messages in thread
From: Christoph Hellwig @ 2026-07-20 9:15 UTC (permalink / raw)
To: Pranjal Shrivastava
Cc: Trond Myklebust, Anna Schumaker, linux-nfs, Chuck Lever,
Jeff Layton, linux-kernel, Christoph Hellwig, Logan Gunthorpe,
Jason Gunthorpe, linux-pci, linux-rdma, Shivaji Kant
On Wed, Jul 15, 2026 at 02:35:38PM +0000, Pranjal Shrivastava wrote:
> Introduce a centralized helper, nfs_release_request_list, to handle
> the bulk release of nfs_page requests from a list.
>
> This serves as a preparatory step for two upcoming improvements:
>
> 1. Pin-Aware Cleanup: As we migrate to iov_iter_extract_* API,
> requests will hold pins (GUP) instead of standard references. The
> helper ensures that the correct unpinning logic gets applied
> consistently across all requests in a list.
>
> 2. Folio Support: In subsequent patches where nfs_page structures
> will cover multi-page folios, this helper provides a clean
> infrastructure to unlock these larger units of I/O in bulk during
> completion, similat to the pattern in bio_release_pages.
>
> Additionally, refactor nfs_read_sync_pgio_error() to utilize this new
> helper.
>
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
> fs/nfs/direct.c | 8 +-------
> fs/nfs/pagelist.c | 18 ++++++++++++++++++
> include/linux/nfs_page.h | 4 ++--
> 3 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
> index 19792a38c924..96995736fac2 100644
> --- a/fs/nfs/direct.c
> +++ b/fs/nfs/direct.c
> @@ -314,13 +314,7 @@ static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)
>
> static void nfs_read_sync_pgio_error(struct list_head *head, int error)
> {
> - struct nfs_page *req;
> -
> - while (!list_empty(head)) {
> - req = nfs_list_entry(head->next);
> - nfs_list_remove_request(req);
> - nfs_release_request(req);
> - }
> + nfs_release_request_list(head);
> }
>
> static void nfs_direct_pgio_init(struct nfs_pgio_header *hdr)
> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
> index 7d51e10fe97a..569bac4faff7 100644
> --- a/fs/nfs/pagelist.c
> +++ b/fs/nfs/pagelist.c
> @@ -622,6 +622,24 @@ void nfs_release_request(struct nfs_page *req)
> }
> EXPORT_SYMBOL_GPL(nfs_release_request);
>
> +/*
> + * nfs_release_request_list - Release a list of NFS read/write requests
> + * @head: list of requests to release
> + *
> + * Removes each request from the list and drops it's refcount.
> + */
> +void nfs_release_request_list(struct list_head *head)
> +{
> + struct nfs_page *req;
> +
> + while (!list_empty(head)) {
> + req = nfs_list_entry(head->next);
req could/should be local here. Or you switch to list_first_or_null,
which some folks prefer.
> +extern void nfs_release_request(struct nfs_page *req);
> +extern void nfs_release_request_list(struct list_head *head);
Please drop the superflous externs for new code.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 3/5] nfs: Introduce nfs_release_request_list helper
2026-07-20 9:15 ` Christoph Hellwig
@ 2026-07-20 9:58 ` Shivaji Kant
2026-07-20 15:03 ` Pranjal Shrivastava
2026-07-20 14:24 ` Pranjal Shrivastava
1 sibling, 1 reply; 17+ messages in thread
From: Shivaji Kant @ 2026-07-20 9:58 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Pranjal Shrivastava, Trond Myklebust, Anna Schumaker, linux-nfs,
Chuck Lever, Jeff Layton, linux-kernel, Logan Gunthorpe,
Jason Gunthorpe, linux-pci, linux-rdma
On Mon, Jul 20, 2026 at 2:45 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Jul 15, 2026 at 02:35:38PM +0000, Pranjal Shrivastava wrote:
> > Introduce a centralized helper, nfs_release_request_list, to handle
> > the bulk release of nfs_page requests from a list.
> >
> > This serves as a preparatory step for two upcoming improvements:
> >
> > 1. Pin-Aware Cleanup: As we migrate to iov_iter_extract_* API,
> > requests will hold pins (GUP) instead of standard references. The
> > helper ensures that the correct unpinning logic gets applied
> > consistently across all requests in a list.
> >
> > 2. Folio Support: In subsequent patches where nfs_page structures
> > will cover multi-page folios, this helper provides a clean
> > infrastructure to unlock these larger units of I/O in bulk during
> > completion, similat to the pattern in bio_release_pages.
nit: similar -> similar
> >
> > Additionally, refactor nfs_read_sync_pgio_error() to utilize this new
> > helper.
> >
> > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > ---
> > fs/nfs/direct.c | 8 +-------
> > fs/nfs/pagelist.c | 18 ++++++++++++++++++
> > include/linux/nfs_page.h | 4 ++--
> > 3 files changed, 21 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
> > index 19792a38c924..96995736fac2 100644
> > --- a/fs/nfs/direct.c
> > +++ b/fs/nfs/direct.c
> > @@ -314,13 +314,7 @@ static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)
> >
> > static void nfs_read_sync_pgio_error(struct list_head *head, int error)
> > {
> > - struct nfs_page *req;
> > -
> > - while (!list_empty(head)) {
> > - req = nfs_list_entry(head->next);
> > - nfs_list_remove_request(req);
> > - nfs_release_request(req);
> > - }
> > + nfs_release_request_list(head);
> > }
> >
> > static void nfs_direct_pgio_init(struct nfs_pgio_header *hdr)
> > diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
> > index 7d51e10fe97a..569bac4faff7 100644
> > --- a/fs/nfs/pagelist.c
> > +++ b/fs/nfs/pagelist.c
> > @@ -622,6 +622,24 @@ void nfs_release_request(struct nfs_page *req)
> > }
> > EXPORT_SYMBOL_GPL(nfs_release_request);
> >
> > +/*
> > + * nfs_release_request_list - Release a list of NFS read/write requests
> > + * @head: list of requests to release
> > + *
> > + * Removes each request from the list and drops it's refcount.
> > + */
> > +void nfs_release_request_list(struct list_head *head)
> > +{
> > + struct nfs_page *req;
> > +
> > + while (!list_empty(head)) {
> > + req = nfs_list_entry(head->next);
>
> req could/should be local here. Or you switch to list_first_or_null,
> which some folks prefer.
>
> > +extern void nfs_release_request(struct nfs_page *req);
> > +extern void nfs_release_request_list(struct list_head *head);
>
> Please drop the superflous externs for new code.
>
Small nit + fix christoph's comment,
else looks ok
Reviewed-by: Shivaji Kant <shivajikant@google.com>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 3/5] nfs: Introduce nfs_release_request_list helper
2026-07-20 9:58 ` Shivaji Kant
@ 2026-07-20 15:03 ` Pranjal Shrivastava
0 siblings, 0 replies; 17+ messages in thread
From: Pranjal Shrivastava @ 2026-07-20 15:03 UTC (permalink / raw)
To: Shivaji Kant
Cc: Christoph Hellwig, Trond Myklebust, Anna Schumaker, linux-nfs,
Chuck Lever, Jeff Layton, linux-kernel, Logan Gunthorpe,
Jason Gunthorpe, linux-pci, linux-rdma
On Mon, Jul 20, 2026 at 03:28:09PM +0530, Shivaji Kant wrote:
> On Mon, Jul 20, 2026 at 2:45 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > On Wed, Jul 15, 2026 at 02:35:38PM +0000, Pranjal Shrivastava wrote:
> > > Introduce a centralized helper, nfs_release_request_list, to handle
> > > the bulk release of nfs_page requests from a list.
> > >
> > > This serves as a preparatory step for two upcoming improvements:
> > >
> > > 1. Pin-Aware Cleanup: As we migrate to iov_iter_extract_* API,
> > > requests will hold pins (GUP) instead of standard references. The
> > > helper ensures that the correct unpinning logic gets applied
> > > consistently across all requests in a list.
> > >
> > > 2. Folio Support: In subsequent patches where nfs_page structures
> > > will cover multi-page folios, this helper provides a clean
> > > infrastructure to unlock these larger units of I/O in bulk during
> > > completion, similat to the pattern in bio_release_pages.
>
> nit: similar -> similar
>
Ack. Fixing it,
> > > +/*
> > > + * nfs_release_request_list - Release a list of NFS read/write requests
> > > + * @head: list of requests to release
> > > + *
> > > + * Removes each request from the list and drops it's refcount.
> > > + */
> > > +void nfs_release_request_list(struct list_head *head)
> > > +{
> > > + struct nfs_page *req;
> > > +
> > > + while (!list_empty(head)) {
> > > + req = nfs_list_entry(head->next);
> >
> > req could/should be local here. Or you switch to list_first_or_null,
> > which some folks prefer.
> >
> > > +extern void nfs_release_request(struct nfs_page *req);
> > > +extern void nfs_release_request_list(struct list_head *head);
> >
> > Please drop the superflous externs for new code.
> >
>
> Small nit + fix christoph's comment,
> else looks ok
>
> Reviewed-by: Shivaji Kant <shivajikant@google.com>
Thanks,
Praan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/5] nfs: Introduce nfs_release_request_list helper
2026-07-20 9:15 ` Christoph Hellwig
2026-07-20 9:58 ` Shivaji Kant
@ 2026-07-20 14:24 ` Pranjal Shrivastava
1 sibling, 0 replies; 17+ messages in thread
From: Pranjal Shrivastava @ 2026-07-20 14:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Trond Myklebust, Anna Schumaker, linux-nfs, Chuck Lever,
Jeff Layton, linux-kernel, Logan Gunthorpe, Jason Gunthorpe,
linux-pci, linux-rdma, Shivaji Kant
On Mon, Jul 20, 2026 at 11:15:05AM +0200, Christoph Hellwig wrote:
> On Wed, Jul 15, 2026 at 02:35:38PM +0000, Pranjal Shrivastava wrote:
> > Introduce a centralized helper, nfs_release_request_list, to handle
> > the bulk release of nfs_page requests from a list.
> >
> > This serves as a preparatory step for two upcoming improvements:
> >
> > 1. Pin-Aware Cleanup: As we migrate to iov_iter_extract_* API,
> > requests will hold pins (GUP) instead of standard references. The
> > helper ensures that the correct unpinning logic gets applied
> > consistently across all requests in a list.
> >
> > 2. Folio Support: In subsequent patches where nfs_page structures
> > will cover multi-page folios, this helper provides a clean
> > infrastructure to unlock these larger units of I/O in bulk during
> > completion, similat to the pattern in bio_release_pages.
> >
> > Additionally, refactor nfs_read_sync_pgio_error() to utilize this new
> > helper.
> >
[...]
> > +/*
> > + * nfs_release_request_list - Release a list of NFS read/write requests
> > + * @head: list of requests to release
> > + *
> > + * Removes each request from the list and drops it's refcount.
> > + */
> > +void nfs_release_request_list(struct list_head *head)
> > +{
> > + struct nfs_page *req;
> > +
> > + while (!list_empty(head)) {
> > + req = nfs_list_entry(head->next);
>
> req could/should be local here. Or you switch to list_first_or_null,
> which some folks prefer.
>
Ack. Made it local
> > +extern void nfs_release_request(struct nfs_page *req);
> > +extern void nfs_release_request_list(struct list_head *head);
>
> Please drop the superflous externs for new code.
>
Ack. Dropping the externs.
Thanks,
Praan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 4/5] nfs: migrate direct I/O to iov_iter_extract_pages
2026-07-15 14:35 [PATCH v3 0/5] nfs: Modernize Direct I/O path Pranjal Shrivastava
` (2 preceding siblings ...)
2026-07-15 14:35 ` [PATCH v3 3/5] nfs: Introduce nfs_release_request_list helper Pranjal Shrivastava
@ 2026-07-15 14:35 ` Pranjal Shrivastava
2026-07-20 9:15 ` Christoph Hellwig
2026-07-15 14:35 ` [PATCH v3 5/5] nfs: introduce nfs_direct_extract_pages helper Pranjal Shrivastava
4 siblings, 1 reply; 17+ messages in thread
From: Pranjal Shrivastava @ 2026-07-15 14:35 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, linux-nfs
Cc: Chuck Lever, Jeff Layton, linux-kernel, Christoph Hellwig,
Logan Gunthorpe, Jason Gunthorpe, linux-pci, linux-rdma,
Shivaji Kant, Pranjal Shrivastava
Migrate the NFS Direct I/O path away from the legacy
iov_iter_get_pages_alloc2() API to the modern iov_iter_extract_pages API.
The transition aligns NFS with the modern VFS extraction model and serves
as a preparatory step for supporting requirements such as page pinning
via GUP for DMA.
The migration fixes a bug in the Direct I/O loop where pages were being
unpinned immediately after request creation. With the new extraction
model, pins are held until the I/O is complete. Manual release in the
loop is correspondingly updated to only clean up failed pages.
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
fs/nfs/direct.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index 96995736fac2..b9ac0a67693c 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -354,16 +354,17 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
inode_dio_begin(inode);
while (iov_iter_count(iter)) {
- struct page **pagevec;
+ struct page **pagevec = NULL;
size_t bytes;
size_t pgbase;
unsigned npages, i;
+ bool pinned = iov_iter_extract_will_pin(iter);
- result = iov_iter_get_pages_alloc2(iter, &pagevec,
- rsize, &pgbase);
+ result = iov_iter_extract_pages(iter, &pagevec,
+ rsize, ~0U, 0, &pgbase);
if (result < 0)
break;
-
+
bytes = result;
npages = (result + pgbase + PAGE_SIZE - 1) / PAGE_SIZE;
for (i = 0; i < npages; i++) {
@@ -371,7 +372,7 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
unsigned int req_len = min_t(size_t, bytes, PAGE_SIZE - pgbase);
/* XXX do we need to do the eof zeroing found in async_filler? */
req = nfs_page_create_from_page(dreq->ctx, pagevec[i],
- false, pgbase, pos,
+ pinned, pgbase, pos,
req_len);
if (IS_ERR(req)) {
result = PTR_ERR(req);
@@ -387,7 +388,8 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
requested_bytes += req_len;
pos += req_len;
}
- nfs_direct_release_pages(pagevec, npages, false);
+ if (i < npages)
+ nfs_direct_release_pages(pagevec + i, npages - i, pinned);
kvfree(pagevec);
if (result < 0)
break;
@@ -891,13 +893,14 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
NFS_I(inode)->write_io += iov_iter_count(iter);
while (iov_iter_count(iter)) {
- struct page **pagevec;
+ struct page **pagevec = NULL;
size_t bytes;
size_t pgbase;
unsigned npages, i;
+ bool pinned = iov_iter_extract_will_pin(iter);
- result = iov_iter_get_pages_alloc2(iter, &pagevec,
- wsize, &pgbase);
+ result = iov_iter_extract_pages(iter, &pagevec,
+ wsize, ~0U, 0, &pgbase);
if (result < 0)
break;
@@ -908,7 +911,7 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
unsigned int req_len = min_t(size_t, bytes, PAGE_SIZE - pgbase);
req = nfs_page_create_from_page(dreq->ctx, pagevec[i],
- false, pgbase, pos,
+ pinned, pgbase, pos,
req_len);
if (IS_ERR(req)) {
result = PTR_ERR(req);
@@ -952,7 +955,8 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
desc.pg_error = 0;
defer = true;
}
- nfs_direct_release_pages(pagevec, npages, false);
+ if (i < npages)
+ nfs_direct_release_pages(pagevec + i, npages - i, pinned);
kvfree(pagevec);
if (result < 0)
break;
--
2.55.0.229.g6434b31f56-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 4/5] nfs: migrate direct I/O to iov_iter_extract_pages
2026-07-15 14:35 ` [PATCH v3 4/5] nfs: migrate direct I/O to iov_iter_extract_pages Pranjal Shrivastava
@ 2026-07-20 9:15 ` Christoph Hellwig
0 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2026-07-20 9:15 UTC (permalink / raw)
To: Pranjal Shrivastava
Cc: Trond Myklebust, Anna Schumaker, linux-nfs, Chuck Lever,
Jeff Layton, linux-kernel, Christoph Hellwig, Logan Gunthorpe,
Jason Gunthorpe, linux-pci, linux-rdma, Shivaji Kant
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 5/5] nfs: introduce nfs_direct_extract_pages helper
2026-07-15 14:35 [PATCH v3 0/5] nfs: Modernize Direct I/O path Pranjal Shrivastava
` (3 preceding siblings ...)
2026-07-15 14:35 ` [PATCH v3 4/5] nfs: migrate direct I/O to iov_iter_extract_pages Pranjal Shrivastava
@ 2026-07-15 14:35 ` Pranjal Shrivastava
2026-07-20 9:15 ` Christoph Hellwig
2026-07-20 10:17 ` Shivaji Kant
4 siblings, 2 replies; 17+ messages in thread
From: Pranjal Shrivastava @ 2026-07-15 14:35 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, linux-nfs
Cc: Chuck Lever, Jeff Layton, linux-kernel, Christoph Hellwig,
Logan Gunthorpe, Jason Gunthorpe, linux-pci, linux-rdma,
Shivaji Kant, Pranjal Shrivastava
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
Refactor nfs_direct_read_schedule_iovec() and
nfs_direct_write_schedule_iovec() to utilize the new helper, unifying
the extraction logic on both paths.
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
fs/nfs/direct.c | 127 ++++++++++++++++++++++++------------------------
1 file changed, 64 insertions(+), 63 deletions(-)
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,
}
}
+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 = iov_iter_extract_will_pin(iter);
+ struct page **pagevec = NULL;
+ ssize_t result, bytes = 0;
+ unsigned int npages, i;
+ size_t pgbase;
+
+ result = iov_iter_extract_pages(iter, &pagevec, size, ~0U, 0, &pgbase);
+ if (result <= 0)
+ return result;
+
+ npages = (result + pgbase + PAGE_SIZE - 1) >> PAGE_SHIFT;
+ for (i = 0; i < npages; i++) {
+ struct nfs_page *req;
+ unsigned int req_len = min_t(size_t, result - bytes, PAGE_SIZE - pgbase);
+
+ req = nfs_page_create_from_page(dreq->ctx, pagevec[i],
+ pinned, pgbase, *pos,
+ req_len);
+ if (IS_ERR(req)) {
+ if (!bytes)
+ bytes = PTR_ERR(req);
+ break;
+ }
+
+ list_add_tail(&req->wb_list, list);
+ pgbase = 0;
+ bytes += req_len;
+ *pos += req_len;
+ }
+
+ if (i < npages) {
+ iov_iter_revert(iter, result - bytes);
+ nfs_direct_release_pages(pagevec + i, npages - i, pinned);
+ }
+
+ kvfree(pagevec);
+ return bytes;
+}
+
void nfs_init_cinfo_from_dreq(struct nfs_commit_info *cinfo,
struct nfs_direct_req *dreq)
{
@@ -346,6 +390,7 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
ssize_t result = -EINVAL;
size_t requested_bytes = 0;
size_t rsize = max_t(size_t, NFS_SERVER(inode)->rsize, PAGE_SIZE);
+ LIST_HEAD(nfs_page_list);
nfs_pageio_init_read(&desc, dreq->inode, false,
&nfs_direct_read_completion_ops);
@@ -354,43 +399,23 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
inode_dio_begin(inode);
while (iov_iter_count(iter)) {
- struct page **pagevec = NULL;
- size_t bytes;
- size_t pgbase;
- unsigned npages, i;
- bool pinned = iov_iter_extract_will_pin(iter);
-
- result = iov_iter_extract_pages(iter, &pagevec,
- rsize, ~0U, 0, &pgbase);
+ result = nfs_direct_extract_pages(dreq, iter, rsize, &pos, &nfs_page_list);
if (result < 0)
break;
- bytes = result;
- npages = (result + pgbase + PAGE_SIZE - 1) / PAGE_SIZE;
- for (i = 0; i < npages; i++) {
- struct nfs_page *req;
- unsigned int req_len = min_t(size_t, bytes, PAGE_SIZE - pgbase);
- /* XXX do we need to do the eof zeroing found in async_filler? */
- req = nfs_page_create_from_page(dreq->ctx, pagevec[i],
- pinned, pgbase, pos,
- req_len);
- if (IS_ERR(req)) {
- result = PTR_ERR(req);
- break;
- }
+ while (!list_empty(&nfs_page_list)) {
+ struct nfs_page *req = nfs_list_entry(nfs_page_list.next);
+ size_t req_len = req->wb_bytes;
+
+ nfs_list_remove_request(req);
if (!nfs_pageio_add_request(&desc, req)) {
result = desc.pg_error;
nfs_release_request(req);
+ nfs_release_request_list(&nfs_page_list);
break;
}
- pgbase = 0;
- bytes -= req_len;
requested_bytes += req_len;
- pos += req_len;
}
- if (i < npages)
- nfs_direct_release_pages(pagevec + i, npages - i, pinned);
- kvfree(pagevec);
if (result < 0)
break;
}
@@ -881,6 +906,7 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
ssize_t result = 0;
size_t requested_bytes = 0;
size_t wsize = max_t(size_t, NFS_SERVER(inode)->wsize, PAGE_SIZE);
+ LIST_HEAD(nfs_page_list);
bool defer = false;
trace_nfs_direct_write_schedule_iovec(dreq);
@@ -893,55 +919,32 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
NFS_I(inode)->write_io += iov_iter_count(iter);
while (iov_iter_count(iter)) {
- struct page **pagevec = NULL;
- size_t bytes;
- size_t pgbase;
- unsigned npages, i;
- bool pinned = iov_iter_extract_will_pin(iter);
-
- result = iov_iter_extract_pages(iter, &pagevec,
- wsize, ~0U, 0, &pgbase);
+ result = nfs_direct_extract_pages(dreq, iter, wsize, &pos, &nfs_page_list);
if (result < 0)
break;
- bytes = result;
- npages = (result + pgbase + PAGE_SIZE - 1) / PAGE_SIZE;
- for (i = 0; i < npages; i++) {
- struct nfs_page *req;
- unsigned int req_len = min_t(size_t, bytes, PAGE_SIZE - pgbase);
-
- req = nfs_page_create_from_page(dreq->ctx, pagevec[i],
- pinned, pgbase, pos,
- req_len);
- if (IS_ERR(req)) {
- result = PTR_ERR(req);
- break;
- }
-
- if (desc.pg_error < 0) {
- nfs_free_request(req);
- result = desc.pg_error;
- break;
- }
-
- pgbase = 0;
- bytes -= req_len;
- requested_bytes += req_len;
- pos += req_len;
+ while (!list_empty(&nfs_page_list)) {
+ struct nfs_page *req = nfs_list_entry(nfs_page_list.next);
+ size_t req_len = req->wb_bytes;
+ nfs_list_remove_request(req);
if (defer) {
nfs_mark_request_commit(req, NULL, &cinfo, 0);
+ requested_bytes += req_len;
continue;
}
nfs_lock_request(req);
- if (nfs_pageio_add_request(&desc, req))
+ if (nfs_pageio_add_request(&desc, req)) {
+ requested_bytes += req_len;
continue;
+ }
/* Exit on hard errors */
if (desc.pg_error < 0 && desc.pg_error != -EAGAIN) {
result = desc.pg_error;
nfs_unlock_and_release_request(req);
+ nfs_release_request_list(&nfs_page_list);
break;
}
@@ -952,12 +955,10 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
spin_unlock(&dreq->lock);
nfs_unlock_request(req);
nfs_mark_request_commit(req, NULL, &cinfo, 0);
+ requested_bytes += req_len;
desc.pg_error = 0;
defer = true;
}
- if (i < npages)
- nfs_direct_release_pages(pagevec + i, npages - i, pinned);
- kvfree(pagevec);
if (result < 0)
break;
}
--
2.55.0.229.g6434b31f56-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 5/5] nfs: introduce nfs_direct_extract_pages helper
2026-07-15 14:35 ` [PATCH v3 5/5] nfs: introduce nfs_direct_extract_pages helper Pranjal Shrivastava
@ 2026-07-20 9:15 ` Christoph Hellwig
2026-07-20 10:17 ` Shivaji Kant
1 sibling, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2026-07-20 9:15 UTC (permalink / raw)
To: Pranjal Shrivastava
Cc: Trond Myklebust, Anna Schumaker, linux-nfs, Chuck Lever,
Jeff Layton, linux-kernel, Christoph Hellwig, Logan Gunthorpe,
Jason Gunthorpe, linux-pci, linux-rdma, Shivaji Kant
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 5/5] nfs: introduce nfs_direct_extract_pages helper
2026-07-15 14:35 ` [PATCH v3 5/5] nfs: introduce nfs_direct_extract_pages helper Pranjal Shrivastava
2026-07-20 9:15 ` Christoph Hellwig
@ 2026-07-20 10:17 ` Shivaji Kant
1 sibling, 0 replies; 17+ messages in thread
From: Shivaji Kant @ 2026-07-20 10:17 UTC (permalink / raw)
To: Pranjal Shrivastava
Cc: Trond Myklebust, Anna Schumaker, linux-nfs, Chuck Lever,
Jeff Layton, linux-kernel, Christoph Hellwig, Logan Gunthorpe,
Jason Gunthorpe, linux-pci, linux-rdma
On Wed, Jul 15, 2026 at 8:05 PM Pranjal Shrivastava <praan@google.com> 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
>
> Refactor nfs_direct_read_schedule_iovec() and
> nfs_direct_write_schedule_iovec() to utilize the new helper, unifying
> the extraction logic on both paths.
>
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
> fs/nfs/direct.c | 127 ++++++++++++++++++++++++------------------------
> 1 file changed, 64 insertions(+), 63 deletions(-)
>
> 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,
> }
> }
>
> +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 = iov_iter_extract_will_pin(iter);
> + struct page **pagevec = NULL;
> + ssize_t result, bytes = 0;
> + unsigned int npages, i;
> + size_t pgbase;
> +
> + result = iov_iter_extract_pages(iter, &pagevec, size, ~0U, 0, &pgbase);
> + if (result <= 0)
> + return result;
> +
> + npages = (result + pgbase + PAGE_SIZE - 1) >> PAGE_SHIFT;
> + for (i = 0; i < npages; i++) {
> + struct nfs_page *req;
> + unsigned int req_len = min_t(size_t, result - bytes, PAGE_SIZE - pgbase);
> +
> + req = nfs_page_create_from_page(dreq->ctx, pagevec[i],
> + pinned, pgbase, *pos,
> + req_len);
> + if (IS_ERR(req)) {
> + if (!bytes)
> + bytes = PTR_ERR(req);
> + break;
> + }
> +
> + list_add_tail(&req->wb_list, list);
> + pgbase = 0;
> + bytes += req_len;
> + *pos += req_len;
> + }
> +
> + if (i < npages) {
> + iov_iter_revert(iter, result - bytes);
> + nfs_direct_release_pages(pagevec + i, npages - i, pinned);
> + }
> +
> + kvfree(pagevec);
> + return bytes;
> +}
> +
> void nfs_init_cinfo_from_dreq(struct nfs_commit_info *cinfo,
> struct nfs_direct_req *dreq)
> {
> @@ -346,6 +390,7 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
> ssize_t result = -EINVAL;
> size_t requested_bytes = 0;
> size_t rsize = max_t(size_t, NFS_SERVER(inode)->rsize, PAGE_SIZE);
> + LIST_HEAD(nfs_page_list);
>
> nfs_pageio_init_read(&desc, dreq->inode, false,
> &nfs_direct_read_completion_ops);
> @@ -354,43 +399,23 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,
> inode_dio_begin(inode);
>
> while (iov_iter_count(iter)) {
> - struct page **pagevec = NULL;
> - size_t bytes;
> - size_t pgbase;
> - unsigned npages, i;
> - bool pinned = iov_iter_extract_will_pin(iter);
> -
> - result = iov_iter_extract_pages(iter, &pagevec,
> - rsize, ~0U, 0, &pgbase);
> + result = nfs_direct_extract_pages(dreq, iter, rsize, &pos, &nfs_page_list);
> if (result < 0)
> break;
>
> - bytes = result;
> - npages = (result + pgbase + PAGE_SIZE - 1) / PAGE_SIZE;
> - for (i = 0; i < npages; i++) {
> - struct nfs_page *req;
> - unsigned int req_len = min_t(size_t, bytes, PAGE_SIZE - pgbase);
> - /* XXX do we need to do the eof zeroing found in async_filler? */
> - req = nfs_page_create_from_page(dreq->ctx, pagevec[i],
> - pinned, pgbase, pos,
> - req_len);
> - if (IS_ERR(req)) {
> - result = PTR_ERR(req);
> - break;
> - }
> + while (!list_empty(&nfs_page_list)) {
> + struct nfs_page *req = nfs_list_entry(nfs_page_list.next);
> + size_t req_len = req->wb_bytes;
> +
> + nfs_list_remove_request(req);
> if (!nfs_pageio_add_request(&desc, req)) {
> result = desc.pg_error;
> nfs_release_request(req);
> + nfs_release_request_list(&nfs_page_list);
> break;
> }
> - pgbase = 0;
> - bytes -= req_len;
> requested_bytes += req_len;
> - pos += req_len;
> }
> - if (i < npages)
> - nfs_direct_release_pages(pagevec + i, npages - i, pinned);
> - kvfree(pagevec);
> if (result < 0)
> break;
> }
> @@ -881,6 +906,7 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
> ssize_t result = 0;
> size_t requested_bytes = 0;
> size_t wsize = max_t(size_t, NFS_SERVER(inode)->wsize, PAGE_SIZE);
> + LIST_HEAD(nfs_page_list);
> bool defer = false;
>
> trace_nfs_direct_write_schedule_iovec(dreq);
> @@ -893,55 +919,32 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
>
> NFS_I(inode)->write_io += iov_iter_count(iter);
> while (iov_iter_count(iter)) {
> - struct page **pagevec = NULL;
> - size_t bytes;
> - size_t pgbase;
> - unsigned npages, i;
> - bool pinned = iov_iter_extract_will_pin(iter);
> -
> - result = iov_iter_extract_pages(iter, &pagevec,
> - wsize, ~0U, 0, &pgbase);
> + result = nfs_direct_extract_pages(dreq, iter, wsize, &pos, &nfs_page_list);
> if (result < 0)
> break;
>
> - bytes = result;
> - npages = (result + pgbase + PAGE_SIZE - 1) / PAGE_SIZE;
> - for (i = 0; i < npages; i++) {
> - struct nfs_page *req;
> - unsigned int req_len = min_t(size_t, bytes, PAGE_SIZE - pgbase);
> -
> - req = nfs_page_create_from_page(dreq->ctx, pagevec[i],
> - pinned, pgbase, pos,
> - req_len);
> - if (IS_ERR(req)) {
> - result = PTR_ERR(req);
> - break;
> - }
> -
> - if (desc.pg_error < 0) {
> - nfs_free_request(req);
> - result = desc.pg_error;
> - break;
> - }
> -
> - pgbase = 0;
> - bytes -= req_len;
> - requested_bytes += req_len;
> - pos += req_len;
> + while (!list_empty(&nfs_page_list)) {
> + struct nfs_page *req = nfs_list_entry(nfs_page_list.next);
> + size_t req_len = req->wb_bytes;
>
> + nfs_list_remove_request(req);
> if (defer) {
> nfs_mark_request_commit(req, NULL, &cinfo, 0);
> + requested_bytes += req_len;
> continue;
> }
>
> nfs_lock_request(req);
> - if (nfs_pageio_add_request(&desc, req))
> + if (nfs_pageio_add_request(&desc, req)) {
> + requested_bytes += req_len;
> continue;
> + }
>
> /* Exit on hard errors */
> if (desc.pg_error < 0 && desc.pg_error != -EAGAIN) {
> result = desc.pg_error;
> nfs_unlock_and_release_request(req);
> + nfs_release_request_list(&nfs_page_list);
> break;
> }
>
> @@ -952,12 +955,10 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,
> spin_unlock(&dreq->lock);
> nfs_unlock_request(req);
> nfs_mark_request_commit(req, NULL, &cinfo, 0);
> + requested_bytes += req_len;
> desc.pg_error = 0;
> defer = true;
> }
> - if (i < npages)
> - nfs_direct_release_pages(pagevec + i, npages - i, pinned);
> - kvfree(pagevec);
> if (result < 0)
> break;
> }
> --
> 2.55.0.229.g6434b31f56-goog
>
Refactoring looks good. Consolidates extraction, error handling, and
iov_iter_revert() into a single helper shared by both read and write
paths.
Reviewed-by: Shivaji Kant <shivajikant@google.com>
^ permalink raw reply [flat|nested] 17+ messages in thread