2022-12-20 16:33:49

by Olga Kornievskaia

Subject: [PATCH 1/1] pNFS/filelayout: Fix coalescing test for single DS

When there is a single DS no striping constraints need to be placed on
the IO. When such constraint is applied then buffered reads don't
coalesce to the DS's rsize.

Signed-off-by: Olga Kornievskaia <[email protected]>
---
fs/nfs/filelayout/filelayout.c | 2 ++
fs/nfs/filelayout/filelayout.h | 1 +
fs/nfs/filelayout/filelayoutdev.c | 4 +++-
3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
index ad34a33b0737..cd819b795935 100644
--- a/fs/nfs/filelayout/filelayout.c
+++ b/fs/nfs/filelayout/filelayout.c
@@ -803,6 +803,8 @@ filelayout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
size = pnfs_generic_pg_test(pgio, prev, req);
if (!size)
return 0;
+ else if (FILELAYOUT_LSEG(pgio->pg_lseg)->single_ds)
+ return size;

/* see if req and prev are in the same stripe */
if (prev) {
diff --git a/fs/nfs/filelayout/filelayout.h b/fs/nfs/filelayout/filelayout.h
index aed0748fd6ec..524920c2cbf8 100644
--- a/fs/nfs/filelayout/filelayout.h
+++ b/fs/nfs/filelayout/filelayout.h
@@ -65,6 +65,7 @@ struct nfs4_filelayout_segment {
struct nfs4_file_layout_dsaddr *dsaddr; /* Point to GETDEVINFO data */
unsigned int num_fh;
struct nfs_fh **fh_array;
+ bool single_ds;
};

struct nfs4_filelayout {
diff --git a/fs/nfs/filelayout/filelayoutdev.c b/fs/nfs/filelayout/filelayoutdev.c
index acf4b88889dc..95ebbe9e7ed4 100644
--- a/fs/nfs/filelayout/filelayoutdev.c
+++ b/fs/nfs/filelayout/filelayoutdev.c
@@ -243,8 +243,10 @@ nfs4_fl_select_ds_fh(struct pnfs_layout_segment *lseg, u32 j)
u32 i;

if (flseg->stripe_type == STRIPE_SPARSE) {
- if (flseg->num_fh == 1)
+ if (flseg->num_fh == 1) {
+ flseg->single_ds = true;
i = 0;
+ }
else if (flseg->num_fh == 0)
/* Use the MDS OPEN fh set in nfs_read_rpcsetup */
return NULL;
--
2.31.1

2022-12-20 16:43:12

by Trond Myklebust

Subject: Re: [PATCH 1/1] pNFS/filelayout: Fix coalescing test for single DS

> On Dec 20, 2022, at 11:29, Olga Kornievskaia <[email protected]> wrote:
>
> When there is a single DS no striping constraints need to be placed on
> the IO. When such constraint is applied then buffered reads don't
> coalesce to the DS's rsize.
>
> Signed-off-by: Olga Kornievskaia <[email protected]>
> ---
> fs/nfs/filelayout/filelayout.c | 2 ++
> fs/nfs/filelayout/filelayout.h | 1 +
> fs/nfs/filelayout/filelayoutdev.c | 4 +++-
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
> index ad34a33b0737..cd819b795935 100644
> --- a/fs/nfs/filelayout/filelayout.c
> +++ b/fs/nfs/filelayout/filelayout.c
> @@ -803,6 +803,8 @@ filelayout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
> size = pnfs_generic_pg_test(pgio, prev, req);
> if (!size)
> return 0;
> + else if (FILELAYOUT_LSEG(pgio->pg_lseg)->single_ds)
> + return size;

Hmm… Instead of adding the boolean, perhaps just add a helper function

static bool filelayout_lseg_is_striped(const struct nfs4_filelayout_segment *flseg)
{
return flseg->num_fh > 1;
}

that can be called here?

>
> /* see if req and prev are in the same stripe */
> if (prev) {
> diff --git a/fs/nfs/filelayout/filelayout.h b/fs/nfs/filelayout/filelayout.h
> index aed0748fd6ec..524920c2cbf8 100644
> --- a/fs/nfs/filelayout/filelayout.h
> +++ b/fs/nfs/filelayout/filelayout.h
> @@ -65,6 +65,7 @@ struct nfs4_filelayout_segment {
> struct nfs4_file_layout_dsaddr *dsaddr; /* Point to GETDEVINFO data */
> unsigned int num_fh;
> struct nfs_fh **fh_array;
> + bool single_ds;
> };
>
> struct nfs4_filelayout {
> diff --git a/fs/nfs/filelayout/filelayoutdev.c b/fs/nfs/filelayout/filelayoutdev.c
> index acf4b88889dc..95ebbe9e7ed4 100644
> --- a/fs/nfs/filelayout/filelayoutdev.c
> +++ b/fs/nfs/filelayout/filelayoutdev.c
> @@ -243,8 +243,10 @@ nfs4_fl_select_ds_fh(struct pnfs_layout_segment *lseg, u32 j)
> u32 i;
>
> if (flseg->stripe_type == STRIPE_SPARSE) {
> - if (flseg->num_fh == 1)
> + if (flseg->num_fh == 1) {
> + flseg->single_ds = true;
> i = 0;
> + }
> else if (flseg->num_fh == 0)
> /* Use the MDS OPEN fh set in nfs_read_rpcsetup */
> return NULL;
> --
> 2.31.1
>



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

2022-12-20 17:39:29

by Olga Kornievskaia

Subject: Re: [PATCH 1/1] pNFS/filelayout: Fix coalescing test for single DS

On Tue, Dec 20, 2022 at 11:37 AM Trond Myklebust
<[email protected]> wrote:
>
>
>
> > On Dec 20, 2022, at 11:29, Olga Kornievskaia <[email protected]> wrote:
> >
> > When there is a single DS no striping constraints need to be placed on
> > the IO. When such constraint is applied then buffered reads don't
> > coalesce to the DS's rsize.
> >
> > Signed-off-by: Olga Kornievskaia <[email protected]>
> > ---
> > fs/nfs/filelayout/filelayout.c | 2 ++
> > fs/nfs/filelayout/filelayout.h | 1 +
> > fs/nfs/filelayout/filelayoutdev.c | 4 +++-
> > 3 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
> > index ad34a33b0737..cd819b795935 100644
> > --- a/fs/nfs/filelayout/filelayout.c
> > +++ b/fs/nfs/filelayout/filelayout.c
> > @@ -803,6 +803,8 @@ filelayout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
> > size = pnfs_generic_pg_test(pgio, prev, req);
> > if (!size)
> > return 0;
> > + else if (FILELAYOUT_LSEG(pgio->pg_lseg)->single_ds)
> > + return size;
>
> Hmm… Instead of adding the boolean, perhaps just add a helper function
>
> static bool filelayout_lseg_is_striped(const struct nfs4_filelayout_segment *flseg)
> {
> return flseg->num_fh > 1;
> }
>
> that can be called here?

Thank you. v2 is on the way.

>
> >
> > /* see if req and prev are in the same stripe */
> > if (prev) {
> > diff --git a/fs/nfs/filelayout/filelayout.h b/fs/nfs/filelayout/filelayout.h
> > index aed0748fd6ec..524920c2cbf8 100644
> > --- a/fs/nfs/filelayout/filelayout.h
> > +++ b/fs/nfs/filelayout/filelayout.h
> > @@ -65,6 +65,7 @@ struct nfs4_filelayout_segment {
> > struct nfs4_file_layout_dsaddr *dsaddr; /* Point to GETDEVINFO data */
> > unsigned int num_fh;
> > struct nfs_fh **fh_array;
> > + bool single_ds;
> > };
> >
> > struct nfs4_filelayout {
> > diff --git a/fs/nfs/filelayout/filelayoutdev.c b/fs/nfs/filelayout/filelayoutdev.c
> > index acf4b88889dc..95ebbe9e7ed4 100644
> > --- a/fs/nfs/filelayout/filelayoutdev.c
> > +++ b/fs/nfs/filelayout/filelayoutdev.c
> > @@ -243,8 +243,10 @@ nfs4_fl_select_ds_fh(struct pnfs_layout_segment *lseg, u32 j)
> > u32 i;
> >
> > if (flseg->stripe_type == STRIPE_SPARSE) {
> > - if (flseg->num_fh == 1)
> > + if (flseg->num_fh == 1) {
> > + flseg->single_ds = true;
> > i = 0;
> > + }
> > else if (flseg->num_fh == 0)
> > /* Use the MDS OPEN fh set in nfs_read_rpcsetup */
> > return NULL;
> > --
> > 2.31.1
> >
>
>
>
> _________________________________
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> [email protected]
>