nfs: modernize direct I/O path
Subsystems (4)
v4 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]/
[v5]
- 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 | 62 ++++++++++++++--- fs/nfs/read.c | 2 +- fs/nfs/write.c | 2 +- include/linux/nfs_page.h | 8 ++- 5 files changed, 130 insertions(+), 83 deletions(-)
base-commit: 5b57716ed7ad2f4b1724461c8777015c4e4befb1
Signature · 1 line
2.55.0.679.g6767b8d81c-goog
PATCH v5 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 | 40 ++++++++++++++++++++++++++++++---------- fs/nfs/read.c | 2 +- fs/nfs/write.c | 2 +- include/linux/nfs_page.h | 3 +++ 5 files changed, 50 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 +30 -10
@@ -404,20 +404,28 @@ 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 +433,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 +444,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 +456,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 +467,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 +477,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 +488,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 +510,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 +566,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 ) {
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.679.g6767b8d81c-goog
PATCH v5 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 | 9 +++++++-- include/linux/nfs_page.h | 1 + 2 files changed, 8 insertions(+), 2 deletions(-)
fs/nfs/pagelist.c +7 -2
@@ -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,14 +572,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 ;
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.679.g6767b8d81c-goog
PATCH v5 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
@@ -624,6 +624,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.679.g6767b8d81c-goog
PATCH v5 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 | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-)
fs/nfs/direct.c +16 -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);
@@ -387,7 +382,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 +887,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 +905,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 +949,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.679.g6767b8d81c-goog
PATCH v5 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 | 127 ++++++++++++++++++++++++------------------------ 1 file changed, 64 insertions(+), 63 deletions(-)
fs/nfs/direct.c +64 -63
@@ -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,43 +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); +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 ; }
@@ -875,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);
@@ -887,55 +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; -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 ; }
@@ -946,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.679.g6767b8d81c-goog
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.
[...]
I'm calling out a few typos / non-code mistakes:
1. On the cover letter I missed updating the subject to v5 for this series (the changelog is noted below).
2. I also accidentally dropped Christoph's Reviewed-by tags on patches 1, 2, and 3 during the rebase (given while reviewing v4 [1]).
Everything else in the series is correct as per the v5 changelog. Please let me know what the right etiquette is? Shall I resend the series?
Apologies for the noise!
Quoted 49 lines
[v5]
- 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[v4]
- https://lore.kernel.org/all/[email protected]/
- Dropped superfluous externs ininclude/linux/nfs_page.h
- Scopedreqlocally inside the loop innfs_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
- Foldedrequested_bytesaccounting in patch 5 due to dropped folio support
- Rebased on fs-next[v2]
- https://lore.kernel.org/all/[email protected]/
- Fix data corruption innfs_direct_extract_pages()by correctly calculating intra-page offsets usingoffset_in_page().
- Fixrequested_bytesaccounting in direct read/write paths to only increment after successful RPC scheduling.
- Add missing kernel-doc descriptions for the @pinned parameter innfs_page_create_from_page()andnfs_page_create_from_folio().
- Rebase on fs-next/[v1] https://lore.kernel.org/all/[email protected]/
Pranjal Shrivastava (5):
nfs: makenfs_pagepin-aware
nfs: track number of pinned pages innfs_page
nfs: introducenfs_release_request_listhelper
nfs: migrate direct I/O toiov_iter_extract_pages
nfs: introducenfs_direct_extract_pageshelperfs/nfs/direct.c | 139 ++++++++++++++++++++------------------- fs/nfs/pagelist.c | 62 ++++++++++++++--- fs/nfs/read.c | 2 +- fs/nfs/write.c | 2 +- include/linux/nfs_page.h | 8 ++- 5 files changed, 130 insertions(+), 83 deletions(-)
Quoted 4 lines
base-commit:
5b57716ed7ad2f4b1724461c8777015c4e4befb1Signature · 1 line
2.55.0.679.g6767b8d81c-goog
Thanks,
Praan
Quoted 20 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.[...]
I'm calling out a few typos / non-code mistakes:
1. On the cover letter I missed updating the subject to v5 for this series (the changelog is noted below).
Quoted 8 lines
2. I also accidentally dropped Christoph's Reviewed-by tags on patches 1, 2, and 3 during the rebase (given while reviewing v4 [1]).
Everything else in the series is correct as per the v5 changelog. Please let me know what the right etiquette is? Shall I resend the series?
Apologies for the noise!
Quick update:
I've posted a v6 [1] with fixes for unpin bugs reported by Sashiko [2] and with R-bs from Christoph that were dropped mistakenly in v5.
Thanks,
Praan
[1] https://lore.kernel.org/all/[email protected]/
[2] https://sashiko.dev/#/patchset/20260810195040.2877784-1-praan%40google.com