2024-05-07 15:16:53

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH 1/1] pNFS/filelayout: fixup pNfs allocation modes

From: Olga Kornievskaia <[email protected]>

Change left over allocation flags.

Fixes: a245832aaa99 ("pNFS/files: Ensure pNFS allocation modes are consistent with nfsiod")
Signed-off-by: Olga Kornievskaia <[email protected]>
---
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



2024-05-08 15:36:01

by Benjamin Coddington

[permalink] [raw]
Subject: Re: [PATCH 1/1] pNFS/filelayout: fixup pNfs allocation modes

On 7 May 2024, at 11:15, Olga Kornievskaia wrote:

> From: Olga Kornievskaia <[email protected]>
>
> Change left over allocation flags.
>
> Fixes: a245832aaa99 ("pNFS/files: Ensure pNFS allocation modes are consistent with nfsiod")
> Signed-off-by: Olga Kornievskaia <[email protected]>
> ---
> 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


2024-05-08 17:15:55

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH 1/1] pNFS/filelayout: fixup pNfs allocation modes

On Wed, May 8, 2024 at 11:25 AM Benjamin Coddington <[email protected]> wrote:
>
> On 7 May 2024, at 11:15, Olga Kornievskaia wrote:
>
> > From: Olga Kornievskaia <[email protected]>
> >
> > Change left over allocation flags.
> >
> > Fixes: a245832aaa99 ("pNFS/files: Ensure pNFS allocation modes are consistent with nfsiod")
> > Signed-off-by: Olga Kornievskaia <[email protected]>
> > ---
> > 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
>

2024-05-08 18:42:24

by Benjamin Coddington

[permalink] [raw]
Subject: Re: [PATCH 1/1] pNFS/filelayout: fixup pNfs allocation modes

On 8 May 2024, at 13:13, Olga Kornievskaia wrote:

> On Wed, May 8, 2024 at 11:25 AM Benjamin Coddington <[email protected]> wrote:
>>
>> On 7 May 2024, at 11:15, Olga Kornievskaia wrote:
>>
>>> From: Olga Kornievskaia <[email protected]>
>>>
>>> Change left over allocation flags.
>>>
>>> Fixes: a245832aaa99 ("pNFS/files: Ensure pNFS allocation modes are consistent with nfsiod")
>>> Signed-off-by: Olga Kornievskaia <[email protected]>
>>> ---
>>> 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 <[email protected]>

Ben