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