Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <49ebb356-0a0d-7ec0-3d6a-4fb79272c41f@wanadoo.fr>
Date:   Fri, 21 Jul 2023 20:14:03 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH] nfs/blocklayout: Use the passed in gfp flags
Content-Language: fr
From:   Marion & Christophe JAILLET <christophe.jaillet@wanadoo.fr>
To:     Dan Carpenter <dan.carpenter@linaro.org>,
        Anna Schumaker <anna@kernel.org>
Cc:     Trond Myklebust <trond.myklebust@hammerspace.com>,
        Jens Axboe <axboe@kernel.dk>, Jack Wang <jinpu.wang@ionos.com>,
        Dave Chinner <dchinner@redhat.com>,
        Hannes Reinecke <hare@suse.de>,
        Christian Brauner <brauner@kernel.org>,
        linux-nfs@vger.kernel.org, kernel-janitors@vger.kernel.org,
        Christoph Hellwig <hch@lst.de>
References: <e655db6f-471b-4184-8907-0551e909acbb@moroto.mountain>
 <CAFX2JfmdpaRWbBypM+Xd4omd86mAbMNQ79+=xAtJXNjip95Sag@mail.gmail.com>
 <e0e9d80e-0e69-e685-c2d5-a658a173b9ce@wanadoo.fr>
In-Reply-To: <e0e9d80e-0e69-e685-c2d5-a658a173b9ce@wanadoo.fr>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


Le 21/07/2023 à 19:59, Christophe JAILLET a écrit :
> Le 21/07/2023 à 19:28, Anna Schumaker a écrit :
>> Hi Dan,
>>
>> On Fri, Jul 21, 2023 at 10:58 AM Dan Carpenter 
>> <dan.carpenter@linaro.org> wrote:
>>>
>>> This allocation should use the passed in GFP_ flags instead of
>>> GFP_KERNEL.  All the callers that I reviewed passed GFP_KERNEL as the
>>> allocation flags so this might not affect runtime, but it's still worth
>>> cleaning up.
>>
>> If all the callers are passing GFP_KERNEL anyway, then can we instead
>> remove the gfp_mask argument from these functions?
> 
> Hi,
> 
> I won't be able to remind the (verrrrrryyyyy long) call chain, but I 
> managed to arrive up to [1].
> So, *if I'm right*, 'gfp_mask' is NOT always GFP_KERNEL.

I can't remind the call chain myself, but my browser history can.

gfp_mask is propagated in all the below functions:

filelayout_pg_init_write()
--> fl_pnfs_update_layout(..., GFP_NOFS)
--> filelayout_check_deviceid()
--> nfs4_find_get_deviceid()
--> nfs4_get_device_info()
--> alloc_deviceid_node()	function pointer in struct pnfs_layoutdriver_type
--> bl_alloc_deviceid_node()
--> bl_parse_deviceid()
--> bl_parse_slice()  /  bl_parse_concat()

CJ

> 
> [1]: 
> https://elixir.bootlin.com/linux/v6.5-rc1/source/fs/nfs/filelayout/filelayout.c#L904
> 
> Just my 2c,
> 
> CJ
> 
>>
>> Anna
>>
>>>
>>> Fixes: 5c83746a0cf2 ("pnfs/blocklayout: in-kernel GETDEVICEINFO XDR 
>>> parsing")
>>> Signed-off-by: Dan Carpenter 
>>> <dan.carpenter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> ---
>>>   fs/nfs/blocklayout/dev.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c
>>> index 70f5563a8e81..65cbb5607a5f 100644
>>> --- a/fs/nfs/blocklayout/dev.c
>>> +++ b/fs/nfs/blocklayout/dev.c
>>> @@ -404,7 +404,7 @@ bl_parse_concat(struct nfs_server *server, struct 
>>> pnfs_block_dev *d,
>>>          int ret, i;
>>>
>>>          d->children = kcalloc(v->concat.volumes_count,
>>> -                       sizeof(struct pnfs_block_dev), GFP_KERNEL);
>>> +                       sizeof(struct pnfs_block_dev), gfp_mask);
>>>          if (!d->children)
>>>                  return -ENOMEM;
>>>
>>> @@ -433,7 +433,7 @@ bl_parse_stripe(struct nfs_server *server, struct 
>>> pnfs_block_dev *d,
>>>          int ret, i;
>>>
>>>          d->children = kcalloc(v->stripe.volumes_count,
>>> -                       sizeof(struct pnfs_block_dev), GFP_KERNEL);
>>> +                       sizeof(struct pnfs_block_dev), gfp_mask);
>>>          if (!d->children)
>>>                  return -ENOMEM;
>>>
>>> -- 
>>> 2.39.2
>>>
>>
> 
>