2023-01-20 05:10:23

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 00/18] Initial conversion of NFS basic I/O to use folios

From: Trond Myklebust <[email protected]>

This set of patches represents an initial effort to convert the page
cache I/O in the NFS client to use native folio functionality. It should
allow the nfs_page structs and their helpers to carry folios (including
folios of order > 0) and to pass their data contents through to the RPC
layer.
Note that because O_DIRECT uses pages, we still need to support the
traditional page based I/O, and so the new struct nfs_page will carry
both types.
I did not touch the fscache code, but I expect that to be able to
continue to work with order 0 folios.

The plan is to merge this functionality with order 0 folios first, in
order to catch any regressions in existing functionality. Then we can
enable order n > 0 once we're happy about the stability (at least for
the non-fscache case).

At this point, the xfstests are all passing without any regressions on
my setup, so I'm throwing the patches over the fence to allow for wider
testing.
Please make sure, in particular to test pNFS if your server supports it.
I didn't have to make any changes to the pNFS code, and I don't expect
any trouble, but it would be good to have validation of that assumption.

---
v2:
- Fix a bisectability issue reported by Anna
- Remove an unnecessary NULL pointer check in nfs_read_folio()

Trond Myklebust (18):
NFS: Fix for xfstests generic/208
NFS: Add basic functionality for tracking folios in struct nfs_page
NFS: Support folios in nfs_generic_pgio()
NFS: Fix nfs_coalesce_size() to work with folios
NFS: Add a helper to convert a struct nfs_page into an inode
NFS: Convert the remaining pagelist helper functions to support folios
NFS: Add a helper nfs_wb_folio()
NFS: Convert buffered reads to use folios
NFS: Convert the function nfs_wb_page() to use folios
NFS: Convert buffered writes to use folios
NFS: Remove unused function nfs_wb_page()
NFS: Convert nfs_write_begin/end to use folios
NFS: Fix up nfs_vm_page_mkwrite() for folios
NFS: Clean up O_DIRECT request allocation
NFS: fix up nfs_release_folio() to try to release the page
NFS: Enable tracing of nfs_invalidate_folio() and nfs_launder_folio()
NFS: Improve tracing of nfs_wb_folio()
NFS: Remove unnecessary check in nfs_read_folio()

fs/nfs/direct.c | 12 +-
fs/nfs/file.c | 124 +++++++------
fs/nfs/internal.h | 38 ++--
fs/nfs/nfstrace.h | 58 ++++--
fs/nfs/pagelist.c | 217 +++++++++++++++++-----
fs/nfs/pnfs.h | 10 +-
fs/nfs/pnfs_nfs.c | 18 +-
fs/nfs/read.c | 94 +++++-----
fs/nfs/write.c | 380 ++++++++++++++++++++-------------------
include/linux/nfs_fs.h | 7 +-
include/linux/nfs_page.h | 79 +++++++-
11 files changed, 646 insertions(+), 391 deletions(-)

--
2.39.0


2023-01-20 05:15:20

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 01/18] NFS: Fix for xfstests generic/208

From: Trond Myklebust <[email protected]>

If the same page and data is being used for multiple requests,
then ignore that when the request indicates we're reading from the start
of the page.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/pagelist.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 16be6dae524f..369e4553399a 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -920,6 +920,9 @@ int nfs_generic_pgio(struct nfs_pageio_descriptor *desc,
req = nfs_list_entry(head->next);
nfs_list_move_request(req, &hdr->pages);

+ if (req->wb_pgbase == 0)
+ last_page = NULL;
+
if (!last_page || last_page != req->wb_page) {
pageused++;
if (pageused > pagecount)
--
2.39.0

2023-01-24 16:07:47

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH v2 00/18] Initial conversion of NFS basic I/O to use folios

On Fri, Jan 20, 2023 at 12:10 AM <[email protected]> wrote:
>
> From: Trond Myklebust <[email protected]>
>
> This set of patches represents an initial effort to convert the page
> cache I/O in the NFS client to use native folio functionality. It should
> allow the nfs_page structs and their helpers to carry folios (including
> folios of order > 0) and to pass their data contents through to the RPC
> layer.
> Note that because O_DIRECT uses pages, we still need to support the
> traditional page based I/O, and so the new struct nfs_page will carry
> both types.
> I did not touch the fscache code, but I expect that to be able to
> continue to work with order 0 folios.
>
> The plan is to merge this functionality with order 0 folios first, in
> order to catch any regressions in existing functionality. Then we can
> enable order n > 0 once we're happy about the stability (at least for
> the non-fscache case).
>
> At this point, the xfstests are all passing without any regressions on
> my setup, so I'm throwing the patches over the fence to allow for wider
> testing.
> Please make sure, in particular to test pNFS if your server supports it.
> I didn't have to make any changes to the pNFS code, and I don't expect
> any trouble, but it would be good to have validation of that assumption.

Here's my experience with running with these patches running thru
xfstest's quick group:
Against a linux server: takes about a couple of minutes longer to run
with folio patches (48m with folio, 45 without) but that's just from 1
run with and and without.

Against an ontap server (so pnfs case): total time is higher with
patches: I have a difference of 47m with folio and 38m without.

While I don't believe this is related to folio patches but I need to
increase my vm's size to 4g to have a successful run of xfstest. Tests
generic/531 and generic/460 are problematic with 2G. generic/531 leads
to oom-killer and generic/460 hangs.


>
> ---
> v2:
> - Fix a bisectability issue reported by Anna
> - Remove an unnecessary NULL pointer check in nfs_read_folio()
>
> Trond Myklebust (18):
> NFS: Fix for xfstests generic/208
> NFS: Add basic functionality for tracking folios in struct nfs_page
> NFS: Support folios in nfs_generic_pgio()
> NFS: Fix nfs_coalesce_size() to work with folios
> NFS: Add a helper to convert a struct nfs_page into an inode
> NFS: Convert the remaining pagelist helper functions to support folios
> NFS: Add a helper nfs_wb_folio()
> NFS: Convert buffered reads to use folios
> NFS: Convert the function nfs_wb_page() to use folios
> NFS: Convert buffered writes to use folios
> NFS: Remove unused function nfs_wb_page()
> NFS: Convert nfs_write_begin/end to use folios
> NFS: Fix up nfs_vm_page_mkwrite() for folios
> NFS: Clean up O_DIRECT request allocation
> NFS: fix up nfs_release_folio() to try to release the page
> NFS: Enable tracing of nfs_invalidate_folio() and nfs_launder_folio()
> NFS: Improve tracing of nfs_wb_folio()
> NFS: Remove unnecessary check in nfs_read_folio()
>
> fs/nfs/direct.c | 12 +-
> fs/nfs/file.c | 124 +++++++------
> fs/nfs/internal.h | 38 ++--
> fs/nfs/nfstrace.h | 58 ++++--
> fs/nfs/pagelist.c | 217 +++++++++++++++++-----
> fs/nfs/pnfs.h | 10 +-
> fs/nfs/pnfs_nfs.c | 18 +-
> fs/nfs/read.c | 94 +++++-----
> fs/nfs/write.c | 380 ++++++++++++++++++++-------------------
> include/linux/nfs_fs.h | 7 +-
> include/linux/nfs_page.h | 79 +++++++-
> 11 files changed, 646 insertions(+), 391 deletions(-)
>
> --
> 2.39.0
>

2023-01-24 16:42:14

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH v2 00/18] Initial conversion of NFS basic I/O to use folios



> On Jan 24, 2023, at 11:07, Olga Kornievskaia <[email protected]> wrote:
>
> On Fri, Jan 20, 2023 at 12:10 AM <[email protected]> wrote:
>>
>> From: Trond Myklebust <[email protected]>
>>
>> This set of patches represents an initial effort to convert the page
>> cache I/O in the NFS client to use native folio functionality. It should
>> allow the nfs_page structs and their helpers to carry folios (including
>> folios of order > 0) and to pass their data contents through to the RPC
>> layer.
>> Note that because O_DIRECT uses pages, we still need to support the
>> traditional page based I/O, and so the new struct nfs_page will carry
>> both types.
>> I did not touch the fscache code, but I expect that to be able to
>> continue to work with order 0 folios.
>>
>> The plan is to merge this functionality with order 0 folios first, in
>> order to catch any regressions in existing functionality. Then we can
>> enable order n > 0 once we're happy about the stability (at least for
>> the non-fscache case).
>>
>> At this point, the xfstests are all passing without any regressions on
>> my setup, so I'm throwing the patches over the fence to allow for wider
>> testing.
>> Please make sure, in particular to test pNFS if your server supports it.
>> I didn't have to make any changes to the pNFS code, and I don't expect
>> any trouble, but it would be good to have validation of that assumption.
>
> Here's my experience with running with these patches running thru
> xfstest's quick group:
> Against a linux server: takes about a couple of minutes longer to run
> with folio patches (48m with folio, 45 without) but that's just from 1
> run with and and without.
>
> Against an ontap server (so pnfs case): total time is higher with
> patches: I have a difference of 47m with folio and 38m without.
>
> While I don't believe this is related to folio patches but I need to
> increase my vm's size to 4g to have a successful run of xfstest. Tests
> generic/531 and generic/460 are problematic with 2G. generic/531 leads
> to oom-killer and generic/460 hangs.


Hmm… Does the performance climb back to normal if you revert the patch 21f5ae90169b ("NFS: fix up nfs_release_folio() to try to release the page”)? That’s the only one I can think of that does more than just convert from struct page -> struct folio.

_________________________________
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]