diff mbox series

[1/1] pNFS/filelayout: fixup pNfs allocation modes

Message ID 20240507151545.26888-1-olga.kornievskaia@gmail.com (mailing list archive)
State New
Headers show
Series [1/1] pNFS/filelayout: fixup pNfs allocation modes | expand

Commit Message

Olga Kornievskaia May 7, 2024, 3:15 p.m. UTC
From: Olga Kornievskaia <kolga@netapp.com>

Change left over allocation flags.

Fixes: a245832aaa99 ("pNFS/files: Ensure pNFS allocation modes are consistent with nfsiod")
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/filelayout/filelayout.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Benjamin Coddington May 8, 2024, 3:25 p.m. UTC | #1
On 7 May 2024, at 11:15, Olga Kornievskaia wrote:

> From: Olga Kornievskaia <kolga@netapp.com>
>
> Change left over allocation flags.
>
> Fixes: a245832aaa99 ("pNFS/files: Ensure pNFS allocation modes are consistent with nfsiod")
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/filelayout/filelayout.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
> index cc2ed4b5a4fd..85d2dc9bc212 100644
> --- a/fs/nfs/filelayout/filelayout.c
> +++ b/fs/nfs/filelayout/filelayout.c
> @@ -875,7 +875,7 @@ filelayout_pg_init_read(struct nfs_pageio_descriptor *pgio,
>  						      req->wb_bytes,
>  						      IOMODE_READ,
>  						      false,
> -						      GFP_KERNEL);
> +						      nfs_io_gfp_mask());
>  		if (IS_ERR(pgio->pg_lseg)) {
>  			pgio->pg_error = PTR_ERR(pgio->pg_lseg);
>  			pgio->pg_lseg = NULL;
> @@ -899,7 +899,7 @@ filelayout_pg_init_write(struct nfs_pageio_descriptor *pgio,
>  						      req->wb_bytes,
>  						      IOMODE_RW,
>  						      false,
> -						      GFP_NOFS);
> +						      nfs_io_gfp_mask());
>  		if (IS_ERR(pgio->pg_lseg)) {
>  			pgio->pg_error = PTR_ERR(pgio->pg_lseg);
>  			pgio->pg_lseg = NULL;
> -- 
> 2.39.1

Looks fine, but I didn't think you could get here from rpciod/nfsiod
context.  I might be missing something, how did you get here from there?

Ben
Olga Kornievskaia May 8, 2024, 5:13 p.m. UTC | #2
On Wed, May 8, 2024 at 11:25 AM Benjamin Coddington <bcodding@redhat.com> wrote:
>
> On 7 May 2024, at 11:15, Olga Kornievskaia wrote:
>
> > From: Olga Kornievskaia <kolga@netapp.com>
> >
> > Change left over allocation flags.
> >
> > Fixes: a245832aaa99 ("pNFS/files: Ensure pNFS allocation modes are consistent with nfsiod")
> > Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> > ---
> >  fs/nfs/filelayout/filelayout.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
> > index cc2ed4b5a4fd..85d2dc9bc212 100644
> > --- a/fs/nfs/filelayout/filelayout.c
> > +++ b/fs/nfs/filelayout/filelayout.c
> > @@ -875,7 +875,7 @@ filelayout_pg_init_read(struct nfs_pageio_descriptor *pgio,
> >                                                     req->wb_bytes,
> >                                                     IOMODE_READ,
> >                                                     false,
> > -                                                   GFP_KERNEL);
> > +                                                   nfs_io_gfp_mask());
> >               if (IS_ERR(pgio->pg_lseg)) {
> >                       pgio->pg_error = PTR_ERR(pgio->pg_lseg);
> >                       pgio->pg_lseg = NULL;
> > @@ -899,7 +899,7 @@ filelayout_pg_init_write(struct nfs_pageio_descriptor *pgio,
> >                                                     req->wb_bytes,
> >                                                     IOMODE_RW,
> >                                                     false,
> > -                                                   GFP_NOFS);
> > +                                                   nfs_io_gfp_mask());
> >               if (IS_ERR(pgio->pg_lseg)) {
> >                       pgio->pg_error = PTR_ERR(pgio->pg_lseg);
> >                       pgio->pg_lseg = NULL;
> > --
> > 2.39.1
>
> Looks fine, but I didn't think you could get here from rpciod/nfsiod
> context.  I might be missing something, how did you get here from there?

I have to admit I don't fully understand (if at all) the implications
of having the wrong flags. I also don't follow what you mean by this
code not being executed by the rpciod/nfsiod context. This code is
done while doing (buffered) IO and performed by the rpciod context?

In truth I was making it consistent with what flexfiles is doing for
their pnfs_update_layout() usage.

>
> Ben
>
Benjamin Coddington May 8, 2024, 6:33 p.m. UTC | #3
On 8 May 2024, at 13:13, Olga Kornievskaia wrote:

> On Wed, May 8, 2024 at 11:25 AM Benjamin Coddington <bcodding@redhat.com> wrote:
>>
>> On 7 May 2024, at 11:15, Olga Kornievskaia wrote:
>>
>>> From: Olga Kornievskaia <kolga@netapp.com>
>>>
>>> Change left over allocation flags.
>>>
>>> Fixes: a245832aaa99 ("pNFS/files: Ensure pNFS allocation modes are consistent with nfsiod")
>>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>>> ---
>>>  fs/nfs/filelayout/filelayout.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
>>> index cc2ed4b5a4fd..85d2dc9bc212 100644
>>> --- a/fs/nfs/filelayout/filelayout.c
>>> +++ b/fs/nfs/filelayout/filelayout.c
>>> @@ -875,7 +875,7 @@ filelayout_pg_init_read(struct nfs_pageio_descriptor *pgio,
>>>                                                     req->wb_bytes,
>>>                                                     IOMODE_READ,
>>>                                                     false,
>>> -                                                   GFP_KERNEL);
>>> +                                                   nfs_io_gfp_mask());
>>>               if (IS_ERR(pgio->pg_lseg)) {
>>>                       pgio->pg_error = PTR_ERR(pgio->pg_lseg);
>>>                       pgio->pg_lseg = NULL;
>>> @@ -899,7 +899,7 @@ filelayout_pg_init_write(struct nfs_pageio_descriptor *pgio,
>>>                                                     req->wb_bytes,
>>>                                                     IOMODE_RW,
>>>                                                     false,
>>> -                                                   GFP_NOFS);
>>> +                                                   nfs_io_gfp_mask());
>>>               if (IS_ERR(pgio->pg_lseg)) {
>>>                       pgio->pg_error = PTR_ERR(pgio->pg_lseg);
>>>                       pgio->pg_lseg = NULL;
>>> --
>>> 2.39.1
>>
>> Looks fine, but I didn't think you could get here from rpciod/nfsiod
>> context.  I might be missing something, how did you get here from there?
>
> I have to admit I don't fully understand (if at all) the implications
> of having the wrong flags. I also don't follow what you mean by this
> code not being executed by the rpciod/nfsiod context. This code is
> done while doing (buffered) IO and performed by the rpciod context?

I was thrown off by the Fixes: tag. The nfs_io_gfp_mask() doesn't have to do
with that context per se, but rather if we're in writeback due to memory
pressure.  That's what the PF_WQ_WORKER check is for which Trond explains
this commit 515dcdcd4873.

> In truth I was making it consistent with what flexfiles is doing for
> their pnfs_update_layout() usage.

Gotcha, makes sense to me now, thanks for helping me.

FWIW:
Reviewed-by: Benjamin Coddington <bcodding@redhat.com>

Ben
diff mbox series

Patch

diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
index cc2ed4b5a4fd..85d2dc9bc212 100644
--- a/fs/nfs/filelayout/filelayout.c
+++ b/fs/nfs/filelayout/filelayout.c
@@ -875,7 +875,7 @@  filelayout_pg_init_read(struct nfs_pageio_descriptor *pgio,
 						      req->wb_bytes,
 						      IOMODE_READ,
 						      false,
-						      GFP_KERNEL);
+						      nfs_io_gfp_mask());
 		if (IS_ERR(pgio->pg_lseg)) {
 			pgio->pg_error = PTR_ERR(pgio->pg_lseg);
 			pgio->pg_lseg = NULL;
@@ -899,7 +899,7 @@  filelayout_pg_init_write(struct nfs_pageio_descriptor *pgio,
 						      req->wb_bytes,
 						      IOMODE_RW,
 						      false,
-						      GFP_NOFS);
+						      nfs_io_gfp_mask());
 		if (IS_ERR(pgio->pg_lseg)) {
 			pgio->pg_error = PTR_ERR(pgio->pg_lseg);
 			pgio->pg_lseg = NULL;