nfs: modernize direct I/O path
Subsystems (4)
v6 cover nfs: modernize direct I/O path
Modernize the NFS Direct I/O path as a preparatory step to enable PCI Peer-to-Peer DMA (P2PDMA) support. Following feedback on the initial RFC [1], the modernization and architectural changes are split into different series. Additionally, based on the discussion in the v2 [2] of this series, the migration of NFS Direct I/O to folios would be handled in a separate follow-up series.
Currently, NFS O_DIRECT relies on the legacy iov_iter_get_pages_alloc2() API which does not support the pinning requirements for P2P memory. The implementation moves NFS to the modern iov_iter_extract_pages() API.
Design
======
1. Pin-Awareness
Standard NFS requests use get_page() and put_page() for memory management. However, memory extracted via iov_iter_extract_pages() requires explicit pinning.
Introduce a PG_PINNED flag and a wb_nr_pinned count to struct nfs_page. This allows the request lifecycle to track ownership of physical pins and ensure that unpinning is performed only when the I/O is complete.
2. API Migration
Migrate the Direct I/O path to the modern iov_iter_extract_pages() API. This aligns NFS with the modern extraction model and serves as the foundation for passing ITER_ALLOW_P2PDMA in a follow-up series.
Upcoming Work / Roadmap
================================
As decided in the RFC [1] & v2 [2], there will be separate series for P2PDMA Enablement and Migrating NFS Direct I/O to use folios.
This series lays the necessary groundwork for the upcoming work. Following this, two additional series are planned:
1. Migrating NFS Direct I/O to Folios
A series that introduces and exports helper from iov_iter.c to allow the nfs_direct_extract_pages() helper introduced here to aggregate multiple pages into a single large folio-based request, aiming to reduce the RPC overhead for hugepage I/O.
2. P2PDMA Enablement for NFS
Enabling ITER_ALLOW_P2PDMA for the Direct I/O path and introducing transport-level negotiation (discovery of P2P-capable RDMA/NVMe devices). This will build upon the PG_PINNED infrastructure introduced by this series.
Testing
=======
This series has been tested with xfstests [3] on RDMA & TCP transports by running the quick test suite for each transport vs. version combo:
./check -g quick -s rdma3 -s rdma40 -s rdma41 -s rdma42 -s tcp3 -s tcp40 -s tcp41 -s tcp42
The tests were run before & after applying the series. No regressions were observed.
The following summary was tabulated via a custom script [4] (on github) to depict that the tests failing in v1 [5] are now passing.
python3 display.py results/*/check.log
+--------------+--------------+--------------+--------------+--------------+--------------+--------------+--------------+--------------+ | testcase | rdma-sys-3 | rdma-sys-4.0 | rdma-sys-4.1 | rdma-sys-4.2 | tcp-sys-3 | tcp-sys-4.0 | tcp-sys-4.1 | tcp-sys-4.2 | +--------------+--------------+--------------+--------------+--------------+--------------+--------------+--------------+--------------+ | generic/091 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/130 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/139 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/143 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/154 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/155 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/183 | skipped | skipped | skipped | pass | skipped | skipped | skipped | pass | | generic/188 | skipped | skipped | skipped | pass | skipped | skipped | skipped | pass | | generic/190 | skipped | skipped | skipped | pass | skipped | skipped | skipped | pass | | generic/196 | skipped | skipped | skipped | pass | skipped | skipped | skipped | pass | | generic/198 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/203 | skipped | skipped | skipped | pass | skipped | skipped | skipped | pass | | generic/214 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/240 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/263 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/287 | skipped | skipped | skipped | pass | skipped | skipped | skipped | pass | | generic/290 | skipped | skipped | skipped | pass | skipped | skipped | skipped | pass | | generic/292 | skipped | skipped | skipped | pass | skipped | skipped | skipped | pass | | generic/330 | skipped | skipped | skipped | pass | skipped | skipped | skipped | pass | | generic/444 | skipped | skipped | skipped | skipped | skipped | skipped | skipped | skipped | | generic/450 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/451 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/586 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/647 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/708 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/729 | pass | pass | pass | pass | pass | pass | pass | pass | | generic/760 | pass | pass | pass | pass | pass | pass | pass | pass | +--------------+--------------+--------------+--------------+--------------+--------------+--------------+--------------+--------------+
Thanks,
Praan
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
[4] https://github.com/pran005/tools/blob/main/display.py
[5] https://lore.kernel.org/all/[email protected]/
[v6]
- Fixed double unpin bugs pointed out by Sashiko:
https://sashiko.dev/#/patchset/20260810195040.2877784-1-praan%40google.com
- Added R-bs from Christoph (due since v4)
[v5]
- https://lore.kernel.org/all/[email protected]/
- Fixed an iov_iter_revert oops in nfs_direct_extract_pages().
- Fixed a refcount underflow in nfs_direct_release_pages() during swap-over-NFS by ensuring we do not call put_page() on unpinned memory.
- Rebased on fs-next
[v4]
- https://lore.kernel.org/all/[email protected]/
- Dropped superfluous externs in include/linux/nfs_page.h
- Scoped req locally inside the loop in nfs_release_request_list()
- Wrapped >80 character lines in Patch 1 to adhere to coding style.
- Fixed typos and capitalization convention in the commit msgs
- Collected Reviewed-by tags from Christoph and Shivaji
- Rebased on fs-next
[v3]
- https://lore.kernel.org/all/[email protected]/
- Dropped patches that added folio support for NFS Direct I/O
- Folded requested_bytes accounting in patch 5 due to dropped folio support
- Rebased on fs-next
[v2]
- https://lore.kernel.org/all/[email protected]/
- Fix data corruption in nfs_direct_extract_pages() by correctly calculating intra-page offsets using offset_in_page().
- Fix requested_bytes accounting in direct read/write paths to only increment after successful RPC scheduling.
- Add missing kernel-doc descriptions for the @pinned parameter in nfs_page_create_from_page() and nfs_page_create_from_folio().
- Rebase on fs-next/
[v1] https://lore.kernel.org/all/[email protected]/
Pranjal Shrivastava (5):
nfs: make nfs_page pin-aware
nfs: track number of pinned pages in nfs_page
nfs: introduce nfs_release_request_list helper
nfs: migrate direct I/O to iov_iter_extract_pages
nfs: introduce nfs_direct_extract_pages helper
fs/nfs/direct.c | 139 ++++++++++++++++++++------------------- fs/nfs/pagelist.c | 66 ++++++++++++++++--- fs/nfs/read.c | 2 +- fs/nfs/write.c | 2 +- include/linux/nfs_page.h | 8 ++- 5 files changed, 134 insertions(+), 83 deletions(-)
base-commit: 15db403711f3cd9b7b764716ddf5909091b604bf
Signature · 1 line
2.55.0.691.gc56d675ccc-goog
PATCH v6 1/5 nfs: make nfs_page pin-aware
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.
--- fs/nfs/direct.c | 22 +++++++++++++------- fs/nfs/pagelist.c | 44 +++++++++++++++++++++++++++++++--------- fs/nfs/read.c | 2 +- fs/nfs/write.c | 2 +- include/linux/nfs_page.h | 3 +++ 5 files changed, 54 insertions(+), 19 deletions(-)
fs/nfs/direct.c +15 -7
@@ -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 ;
fs/nfs/pagelist.c +34 -10
112 lines not on this page · raw message
fs/nfs/read.c +1 -1
@@ -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))
fs/nfs/write.c +1 -1
@@ -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);
include/linux/nfs_page.h +3 -0
@@ -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 *);
Signature · 1 line
2.55.0.691.gc56d675ccc-goog
[email protected]
AI reviewRe: [PATCH v6 1/5] nfs: make nfs_page pin-aware
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Subrequests incorrectly drop GUP pins multiple times because `req == req->wb_head` evaluates to…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Subrequests incorrectly drop GUP pins multiple times because req == req->wb_head evaluates to true for all requests during teardown, causing pin count underflow and Use-After-Free.
- [Medium] Hardcoded unpin count of 1 in unpin_user_folio() leaks GUP pins for large folios spanning multiple pages.
--
--- Patch [1]: [PATCH v6 1/5] nfs: make nfs_page pin-aware --- commit 21b0f62730c29dca3085e0b77d441b62786919e8
Author: Pranjal Shrivastava <[email protected]>
nfs: make nfs_page pin-aware
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.
Quoted 4 lines
fs/nfs/pagelist.c +0
-0
[ ... ]
fs/nfs/pagelist.c static void nfs_clear_request(struct nfs_page *req) · line 571
Quoted 8 lines
fs/nfs/pagelist.c +3
-1@@ -552,11 +566,21 @@ 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)) { +if (req == req->wb_head) +unpin_user_folio (folio,1 );
[Severity: Medium]
Will this leak GUP pins for large folios spanning multiple pages?
If a caller uses nfs_page_create_from_folio() with pinned=true for a folio spanning multiple pages, hardcoding 1 releases only one pin and leaks the rest.
Also, does this check for the head request evaluate to true for all subrequests?
During teardown, nfs_page_group_destroy() breaks the page group linkage by setting tmp->wb_head = tmp before freeing the request:
nfs_page_group_destroy()
tmp->wb_head = tmp;
nfs_free_request(tmp);Because of this, when nfs_clear_request() is called, req == req->wb_head will evaluate to true for all subrequests, causing each subrequest to unconditionally invoke unpin_user_folio() and potentially resulting in a pin count underflow and Use-After-Free.
fs/nfs/pagelist.c static void nfs_clear_request(struct nfs_page *req) · line 580
Quoted 10 lines
fs/nfs/pagelist.c +6
-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)) { +if (req == req->wb_head) +unpin_user_page (page);
[Severity: Medium]
Similar to the issue above, because nfs_page_group_destroy() severs the linkage and sets tmp->wb_head = tmp, will this unconditionally call unpin_user_page() for all subrequests and cause a pin count underflow?
Quoted 5 lines
fs/nfs/pagelist.c +3
-0+ }else { +put_page (page); + } req->wb_page =NULL ; }
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Re: [PATCH v6 1/5] nfs: make nfs_page pin-aware
Quoted 51 lines · On Fri, Aug 14, 2026 at 02:46:57PM +0000, [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Subrequests incorrectly drop GUP pins multiple times becausereq == req->wb_headevaluates to true for all requests during teardown, causing pin count underflow and Use-After-Free.
- [Medium] Hardcoded unpin count of 1 inunpin_user_folio()leaks GUP pins for large folios spanning multiple pages.
----- Patch [1]: [PATCH v6 1/5] nfs: make
nfs_pagepin-aware --- commit21b0f62730c29dca3085e0b77d441b62786919e8
Author: Pranjal Shrivastava <[email protected]>nfs: make
nfs_pagepin-awareModernizing 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_pagemust track whether it holds a pin or a standard reference.Quoted 4 lines
fs/nfs/pagelist.c +0
-0[ ... ]
fs/nfs/pagelist.c static void nfs_clear_request(struct nfs_page *req) · line 571
Quoted 8 lines
fs/nfs/pagelist.c +3
-1@@ -552,11 +566,21 @@ 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)) { +if (req == req->wb_head) +unpin_user_folio (folio,1 );[Severity: Medium]
Will this leak GUP pins for large folios spanning multiple pages?If a caller uses
nfs_page_create_from_folio()with pinned=true for a folio spanning multiple pages, hardcoding 1 releases only one pin and leaks the rest.Also, does this check for the head request evaluate to true for all subrequests?
During teardown,
nfs_page_group_destroy()breaks the page group linkage by settingtmp->wb_head= tmp before freeing the request:
nfs_page_group_destroy()tmp->wb_head = tmp; nfs_free_request(tmp);Because of this, when
nfs_clear_request()is called, req ==req->wb_headwill evaluate to true for all subrequests, causing each subrequest to unconditionally invokeunpin_user_folio()and potentially resulting in a pin count underflow and Use-After-Free.
Quoted 16 lines
fs/nfs/pagelist.c static void nfs_clear_request(struct nfs_page *req) · line 580
Quoted 10 lines
fs/nfs/pagelist.c +6
-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)) { +if (req == req->wb_head) +unpin_user_page (page);[Severity: Medium]
Similar to the issue above, becausenfs_page_group_destroy()severs the linkage and setstmp->wb_head= tmp, will this unconditionally callunpin_user_page()for all subrequests and cause a pin count underflow?
These are fixed by patch 2 with the introduction of wb_nr_pinned.
Thanks,
Praan
PATCH v6 2/5 nfs: track number of pinned pages in nfs_page
In reply to Pranjal ShrivastavaTrack 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.
--- fs/nfs/pagelist.c | 13 +++++++++---- include/linux/nfs_page.h | 1 + 2 files changed, 10 insertions(+), 4 deletions(-)
fs/nfs/pagelist.c +9 -4
@@ -457,6 +457,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);
@@ -489,6 +491,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);
@@ -567,8 +572,8 @@ static void nfs_clear_request(struct nfs_page *req)if (folio !=NULL ) {if (test_and_clear_bit (PG_PINNED , &req->wb_flags)) { -if (req == req->wb_head) -unpin_user_folio (folio,1 ); +if (req->wb_nr_pinned >0 ) +unpin_user_folio (folio, req->wb_nr_pinned); }else {folio_put (folio); }
@@ -576,8 +581,8 @@ static void nfs_clear_request(struct nfs_page *req)clear_bit (PG_FOLIO , &req->wb_flags); }else if (page !=NULL ) {if (test_and_clear_bit (PG_PINNED , &req->wb_flags)) { -if (req == req->wb_head) -unpin_user_page (page); +if (req->wb_nr_pinned >0 ) +unpin_user_pages (&page, req->wb_nr_pinned); }else {put_page (page); }
include/linux/nfs_page.h +1 -0
@@ -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 ;
Signature · 1 line
2.55.0.691.gc56d675ccc-goog
[email protected]
AI reviewRe: [PATCH v6 2/5] nfs: track number of pinned pages in nfs_page
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Latent stack out-of-bounds read by passing a single local stack variable to unpin_user_pages()…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Latent stack out-of-bounds read by passing a single local stack variable to unpin_user_pages() with a dynamically loaded length. --
--- Patch [2]: [PATCH v6 2/5] nfs: track number of pinned pages in nfs_page --- commit 1fad80f498aa716421accabc5c2572235b589709
Author: Pranjal Shrivastava <[email protected]>
nfs: track number of pinned pages in nfs_page
This commit adds wb_nr_pinned to struct nfs_page to track the number of pinned pages. It updates request creation and cleanup helpers to ensure that only primary requests perform the final unpinning operation.
Quoted 4 lines
fs/nfs/pagelist.c +0
-0
[ ... ]
Quoted 12 lines
fs/nfs/pagelist.c +2
-2@@ -567,8 +572,8 @@ static void nfs_clear_request(struct nfs_page *req)if (folio !=NULL ) {if (test_and_clear_bit (PG_PINNED , &req->wb_flags)) { -if (req == req->wb_head) -unpin_user_folio (folio,1 ); +if (req->wb_nr_pinned >0 ) +unpin_user_folio (folio, req->wb_nr_pinned); }else {folio_put (folio); }
fs/nfs/pagelist.c static void nfs_clear_request(struct nfs_page *req) · line 585
Quoted 7 lines
fs/nfs/pagelist.c +2
-2@@ -576,8 +581,8 @@ static void nfs_clear_request(struct nfs_page *req)clear_bit (PG_FOLIO , &req->wb_flags); }else if (page !=NULL ) {if (test_and_clear_bit (PG_PINNED , &req->wb_flags)) { -if (req == req->wb_head) -unpin_user_page (page); +if (req->wb_nr_pinned >0 ) +unpin_user_pages (&page, req->wb_nr_pinned);
[Severity: Medium]
Could this lead to an out-of-bounds stack read if wb_nr_pinned is ever greater than 1?
Looking at nfs_clear_request(), page is declared as a single local pointer on the stack:
static void nfs_clear_request(struct nfs_page *req)
{
...
struct page *page = req->wb_page;If req->wb_nr_pinned exceeds 1, unpin_user_pages() will read adjacent stack memory to interpret as struct page pointers. While wb_nr_pinned is currently hardcoded to 1 when a request is created from a page, passing a dynamically loaded length alongside a pointer to a single local stack variable seems prone to future regressions.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Re: [PATCH v6 2/5] nfs: track number of pinned pages in nfs_page
Quoted 53 lines · On Fri, Aug 14, 2026 at 02:47:13PM +0000, [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Latent stack out-of-bounds read by passing a single local stack variable tounpin_user_pages()with a dynamically loaded length. ----- Patch [2]: [PATCH v6 2/5] nfs: track number of pinned pages in
nfs_page--- commit1fad80f498aa716421accabc5c2572235b589709
Author: Pranjal Shrivastava <[email protected]>nfs: track number of pinned pages in
nfs_pageThis commit adds
wb_nr_pinnedto structnfs_pageto track the number of pinned pages. It updates request creation and cleanup helpers to ensure that only primary requests perform the final unpinning operation.Quoted 4 lines
fs/nfs/pagelist.c +0
-0[ ... ]
Quoted 12 lines
fs/nfs/pagelist.c +2
-2@@ -567,8 +572,8 @@ static void nfs_clear_request(struct nfs_page *req)if (folio !=NULL ) {if (test_and_clear_bit (PG_PINNED , &req->wb_flags)) { -if (req == req->wb_head) -unpin_user_folio (folio,1 ); +if (req->wb_nr_pinned >0 ) +unpin_user_folio (folio, req->wb_nr_pinned); }else {folio_put (folio); }fs/nfs/pagelist.c static void nfs_clear_request(struct nfs_page *req) · line 585
Quoted 7 lines
fs/nfs/pagelist.c +2
-2@@ -576,8 +581,8 @@ static void nfs_clear_request(struct nfs_page *req)clear_bit (PG_FOLIO , &req->wb_flags); }else if (page !=NULL ) {if (test_and_clear_bit (PG_PINNED , &req->wb_flags)) { -if (req == req->wb_head) -unpin_user_page (page); +if (req->wb_nr_pinned >0 ) +unpin_user_pages (&page, req->wb_nr_pinned);[Severity: Medium]
Could this lead to an out-of-bounds stack read ifwb_nr_pinnedis ever greater than 1?Looking at
nfs_clear_request(), page is declared as a single local pointer on the stack:static void
nfs_clear_request(struct nfs_page *req){ ... struct page *page = req->wb_page;
Quoted 6 lines
If
req->wb_nr_pinnedexceeds 1,unpin_user_pages()will read adjacent stack memory to interpret as struct page pointers. Whilewb_nr_pinnedis currently hardcoded to 1 when a request is created from a page, passing a dynamically loaded length alongside a pointer to a single local stack variable seems prone to future regressions.
In this (non-folio) path wb_nr_pinned would always ever be 1. Although, I think we could use unpin_user_page (singular) here.
Thanks,
Praan
Quoted 2 lines
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
PATCH v6 3/5 nfs: introduce nfs_release_request_list helper
In reply to Pranjal ShrivastavaIntroduce 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, similar to the pattern in bio_release_pages.
Additionally, refactor nfs_read_sync_pgio_error() to utilize this new helper.
--- fs/nfs/direct.c | 8 +------- fs/nfs/pagelist.c | 17 +++++++++++++++++ include/linux/nfs_page.h | 4 ++-- 3 files changed, 20 insertions(+), 9 deletions(-)
fs/nfs/direct.c +1 -7
@@ -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)
fs/nfs/pagelist.c +17 -0
@@ -628,6 +628,23 @@ 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) +{ +while (!list_empty (head)) { +struct nfs_page *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
include/linux/nfs_page.h +2 -2
@@ -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 *); - +void nfs_release_request (struct nfs_page *req); +void nfs_release_request_list (struct list_head *head); extern void nfs_pageio_init(struct nfs_pageio_descriptor *desc, struct inode *inode,
Signature · 1 line
2.55.0.691.gc56d675ccc-goog
[email protected]
AI reviewRe: [PATCH v6 3/5] nfs: introduce nfs_release_request_list helper
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 15 lines
Introduce a centralized helper,
nfs_release_request_list, to handle the bulk release ofnfs_pagerequests 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_pagestructures will cover multi-page folios, this helper provides a clean infrastructure to unlock these larger units of I/O in bulk during completion, similar to the pattern inbio_release_pages.
Quoted 4 lines
Additionally, refactor
nfs_read_sync_pgio_error()to utilize this new helper.Signed-off-by Pranjal Shrivastava <[email protected]>
Sashiko has reviewed this patch and found no issues. It looks great!
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
PATCH v6 4/5 nfs: migrate direct I/O to iov_iter_extract_pages
In reply to Pranjal ShrivastavaMigrate 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.
--- fs/nfs/direct.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)
fs/nfs/direct.c +19 -18
@@ -168,14 +168,8 @@ int nfs_swap_rw(struct kiocb *iocb, struct iov_iter *iter)static void nfs_direct_release_pages (struct page **pages,unsigned int npages,bool pinned) { -unsigned int i; - -if (pinned) { +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,
@@ -354,16 +348,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 +366,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);
@@ -380,6 +375,7 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,if (!nfs_pageio_add_request (&desc, req)) { result = desc.pg_error;nfs_release_request (req); + i++;break ; } pgbase =0 ;
@@ -387,7 +383,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 +888,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 +906,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);
@@ -918,6 +916,7 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,if (desc.pg_error <0 ) {nfs_free_request (req); result = desc.pg_error; + i++;break ; }
@@ -939,6 +938,7 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,if (desc.pg_error <0 && desc.pg_error != -EAGAIN ) { result = desc.pg_error;nfs_unlock_and_release_request (req); + i++;break ; }
@@ -952,7 +952,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 ;
Signature · 1 line
2.55.0.691.gc56d675ccc-goog
[email protected]
AI reviewRe: [PATCH v6 4/5] nfs: migrate direct I/O to iov_iter_extract_pages
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] If request creation or insertion fails mid-chunk, the iterator is not reverted for the remaining un-scheduled bytes, leading to iterator desync.
- [High] If iov_iter_extract_pages returns 0, the scheduling loops will spin infinitely without making progress.
--
--- Patch [4]: [PATCH v6 4/5] nfs: migrate direct I/O to iov_iter_extract_pages --- commit 5228f408a5f9a33be680decc8231db65a3427585
Author: Pranjal Shrivastava <[email protected]>
nfs: migrate direct I/O to iov_iter_extract_pages
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.
Quoted 4 lines
fs/nfs/direct.c +0
-0
[ ... ]
Quoted 7 lines
fs/nfs/direct.c +1
-1@@ -348,16 +348,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;
fs/nfs/direct.c static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq, · line 360
Quoted 10 lines
fs/nfs/direct.c +3
-2size_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 ;
[Severity: High]
This is a pre-existing issue, but if iov_iter_extract_pages() returns 0, will this loop spin indefinitely?
The outer loop in nfs_direct_read_schedule_iovec() relies on iov_iter_count(iter) > 0 to terminate. If iov_iter_extract_pages() returns 0 (which can happen if zero pages are pinned), the result < 0 check won't catch it.
This leaves the iterator unadvanced and bytes = 0, causing the loop to repeat indefinitely and potentially leading to a kernel hang.
Should the check be updated to if (result <= 0)?
Quoted 15 lines
fs/nfs/direct.c +2
-2- + bytes = result; npages = (result + pgbase +PAGE_SIZE -1 ) /PAGE_SIZE ;for (i =0 ; i < npages; i++) {@@ -371,7 +366,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);
fs/nfs/direct.c static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq, · line 380
Quoted 6 lines
fs/nfs/direct.c +1
-0@@ -380,6 +375,7 @@ static ssize_t nfs_direct_read_schedule_iovec(struct nfs_direct_req *dreq,if (!nfs_pageio_add_request (&desc, req)) { result = desc.pg_error;nfs_release_request (req); + i++;break ; }
[Severity: High]
This is a pre-existing issue, but does this early break cause the iterator to become desynchronized?
iov_iter_extract_pages() advances the iterator by the total extracted bytes. If the inner loop breaks early here (e.g. because nfs_pageio_add_request() fails), the remaining unprocessed bytes in the chunk are skipped, but the iterator is never reverted for those skipped bytes.
The caller calculates the revert amount based on successfully processed bytes, so the iterator remains incorrectly advanced. If the VFS retries the remaining data, it might resume from the wrong offset and silently skip data.
Does this path need to call iov_iter_revert() for the unprocessed bytes?
[ ... ]
Quoted 7 lines
fs/nfs/direct.c +1
-1@@ -888,13 +888,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;
fs/nfs/direct.c static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq, · line 900
Quoted 10 lines
fs/nfs/direct.c +3
-2size_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 ;
[Severity: High]
This isn't a bug introduced by this patch, but similar to the read path, if iov_iter_extract_pages() returns 0 here, will it result in an infinite loop in nfs_direct_write_schedule_iovec()?
[ ... ]
fs/nfs/direct.c static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq, · line 943
Quoted 7 lines
fs/nfs/direct.c +1
-0@@ -938,6 +938,7 @@ static ssize_t nfs_direct_write_schedule_iovec(struct nfs_direct_req *dreq,if (desc.pg_error <0 && desc.pg_error != -EAGAIN ) { result = desc.pg_error;nfs_unlock_and_release_request (req); + i++;break ; }
[Severity: High]
This is a pre-existing issue, but this write path seems to have the same iterator advancement leak as the read path when breaking early on hard errors. Should it also revert the iterator for the unprocessed bytes?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
PATCH v6 5/5 nfs: introduce nfs_direct_extract_pages helper
In reply to Pranjal ShrivastavaIntroduce 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.
--- fs/nfs/direct.c | 130 ++++++++++++++++++++++++------------------------ 1 file changed, 64 insertions(+), 66 deletions(-)
fs/nfs/direct.c +64 -66
@@ -172,6 +172,50 @@ static void nfs_direct_release_pages(struct page **pages, unsigned int npages,unpin_user_pages (pages, 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 ; +int err =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)) { + err =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 ? bytes : err; +} +void nfs_init_cinfo_from_dreq (struct nfs_commit_info *cinfo,struct nfs_direct_req *dreq) {
@@ -340,6 +384,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);
@@ -348,44 +393,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); - i++; +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 ; }
@@ -876,6 +900,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);
@@ -888,57 +913,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; - i++; -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); - i++; +nfs_release_request_list (&nfs_page_list);break ; }
@@ -949,12 +949,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 ; }
Signature · 1 line
2.55.0.691.gc56d675ccc-goog
[email protected]
AI reviewRe: [PATCH v6 5/5] nfs: introduce nfs_direct_extract_pages helper
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 9 lines
Introduce
nfs_direct_extract_pages()indirect.cto centralize page extraction and request creation for the Direct I/O path. The helper manages extraction from the iters and builds a list ofnfs_pagerequestsRefactor
nfs_direct_read_schedule_iovec()andnfs_direct_write_schedule_iovec()to utilize the new helper, unifying the extraction logic on both paths.Signed-off-by Pranjal Shrivastava <[email protected]>
Sashiko has reviewed this patch and found no issues. It looks great!
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
Quoted 11 lines · Pranjal Shrivastava
Modernize the NFS Direct I/O path as a preparatory step to enable PCI Peer-to-Peer DMA (P2PDMA) support. Following feedback on the initial RFC [1], the modernization and architectural changes are split into different series. Additionally, based on the discussion in the v2 [2] of this series, the migration of NFS Direct I/O to folios would be handled in a separate follow-up series.
Currently, NFS
O_DIRECTrelies on the legacyiov_iter_get_pages_alloc2()API which does not support the pinning requirements for P2P memory. The implementation moves NFS to the moderniov_iter_extract_pages()API.
Gentle ping on this series.
(I realized that I sent it close to the merge window, Apologies).
Please let me know if anything else needs to be done for this..
[...]
Quoted 4 lines
[v6]
- Fixed double unpin bugs pointed out by Sashiko:https://sashiko.dev/#/patchset/20260810195040.2877784-1-praan%40google.com
Quoted 9 lines
- Added R-bs from Christoph (due since v4)
[v5]
- https://lore.kernel.org/all/[email protected]/
- Fixed aniov_iter_revertoops innfs_direct_extract_pages().
- Fixed a refcount underflow innfs_direct_release_pages()during swap-over-NFS by ensuring we do not callput_page()on unpinned memory.
- Rebased on fs-next
[...]
Thanks,
Praan
Anna Schumaker
maintainerHi Pranjal,
Quoted 13 lines · Pranjal Shrivastava
Quoted 11 lines · Pranjal Shrivastava
Modernize the NFS Direct I/O path as a preparatory step to enable PCI Peer-to-Peer DMA (P2PDMA) support. Following feedback on the initial RFC [1], the modernization and architectural changes are split into different series. Additionally, based on the discussion in the v2 [2] of this series, the migration of NFS Direct I/O to folios would be handled in a separate follow-up series.
Currently, NFS
O_DIRECTrelies on the legacyiov_iter_get_pages_alloc2()API which does not support the pinning requirements for P2P memory. The implementation moves NFS to the moderniov_iter_extract_pages()API.
Quoted 4 lines
Gentle ping on this series.
(I realized that I sent it close to the merge window, Apologies).Please let me know if anything else needs to be done for this..
Thanks for the ping! The plan right now is to send it along as part of the 7.4 merge window. I haven't pushed out a public linux-next branch yet, but when I do these patches will be included.
I hope this helps!
Anna
Quoted 21 lines
[...]
Quoted 4 lines
[v6]
- Fixed double unpin bugs pointed out by Sashiko:https://sashiko.dev/#/patchset/20260810195040.2877784-1-praan%40google.comQuoted 9 lines
- Added R-bs from Christoph (due since v4)
[v5]
- https://lore.kernel.org/all/[email protected]/
- Fixed aniov_iter_revertoops innfs_direct_extract_pages().
- Fixed a refcount underflow innfs_direct_release_pages()during swap-over-NFS by ensuring we do not callput_page()on unpinned memory.
- Rebased on fs-next[...]
Thanks,
Praan
Quoted 19 lines · Anna Schumaker
Hi Pranjal,
Quoted 16 lines · Pranjal Shrivastava
Quoted 11 lines · Pranjal Shrivastava
Modernize the NFS Direct I/O path as a preparatory step to enable PCI Peer-to-Peer DMA (P2PDMA) support. Following feedback on the initial RFC [1], the modernization and architectural changes are split into different series. Additionally, based on the discussion in the v2 [2] of this series, the migration of NFS Direct I/O to folios would be handled in a separate follow-up series.
Currently, NFS
O_DIRECTrelies on the legacyiov_iter_get_pages_alloc2()API which does not support the pinning requirements for P2P memory. The implementation moves NFS to the moderniov_iter_extract_pages()API.Gentle ping on this series.
(I realized that I sent it close to the merge window, Apologies).
Quoted 9 lines
Quoted 1 line
Please let me know if anything else needs to be done for this..
Thanks for the ping! The plan right now is to send it along as part of the 7.4 merge window. I haven't pushed out a public linux-next branch yet, but when I do these patches will be included.
I hope this helps!
Anna
Thanks a lot for responding, quite helpful! :)
Best,
Praan