Question about vmalloc(GFP_NOFS)

Question about vmalloc(GFP_NOFS)

[Pavel Tikhomirov]

Hello,

I see that in kernel code we have couple of places where kvmalloc is 
used with GFP_NOFS flag:

git grep kvmalloc.*NOFS
fs/bcachefs/journal_io.c:       new_buf = kvmalloc(new_size, 
GFP_NOFS|__GFP_NOWARN);
fs/ext4/xattr.c:                buffer = kvmalloc(value_size, GFP_NOFS);
fs/f2fs/compress.c:     cc->private = 
f2fs_kvmalloc(F2FS_I_SB(cc->inode), size, GFP_NOFS);
net/ceph/osdmap.c:      state = kvmalloc(array_size(max, 
sizeof(*state)), GFP_NOFS);
net/ceph/osdmap.c:      weight = kvmalloc(array_size(max, 
sizeof(*weight)), GFP_NOFS);
net/ceph/osdmap.c:      addr = kvmalloc(array_size(max, sizeof(*addr)), 
GFP_NOFS);

and with GFP_NOIO flag too:

git grep kvmalloc.*NOIO
drivers/md/dm-integrity.c:      recalc_tags = kvmalloc(recalc_tags_size, 
GFP_NOIO);
drivers/md/dm-ioctl.c:  dmi = kvmalloc(param_kernel->data_size, GFP_NOIO 
| __GFP_HIGH);
net/ceph/messenger_v2.c:        buf = kvmalloc(len, GFP_NOIO);
net/ceph/osdmap.c:      work = kvmalloc(work_size, GFP_NOIO);

And AFAIU documentation 
https://docs.kernel.org/core-api/gfp_mask-from-fs-io.html#what-about-vmalloc-gfp-nofs 
vmalloc allocation with GFP_NOFS may end up doing "GFP_KERNEL 
allocations deep inside the allocator", which can potentially lead to 
deadlock in IO/FS code paths.

Does it mean that we should rework all those paths to memalloc_noio_save 
/ memalloc_noio_restore variant? Or is it already safe to use 
kvmalloc(GFP_NOIO) in modern kernel?

Or maybe I misunderstand something, sorry in advance if that's the case.

-- 
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.

Re: Question about vmalloc(GFP_NOFS)

[Pavel Tikhomirov]

On 11/19/24 13:48, Pavel Tikhomirov wrote:
> Hello,
> 
> I see that in kernel code we have couple of places where kvmalloc is 
> used with GFP_NOFS flag:
> 
> git grep kvmalloc.*NOFS
> fs/bcachefs/journal_io.c:       new_buf = kvmalloc(new_size, GFP_NOFS| 
> __GFP_NOWARN);
> fs/ext4/xattr.c:                buffer = kvmalloc(value_size, GFP_NOFS);
> fs/f2fs/compress.c:     cc->private = f2fs_kvmalloc(F2FS_I_SB(cc- 
>  >inode), size, GFP_NOFS);
> net/ceph/osdmap.c:      state = kvmalloc(array_size(max, 
> sizeof(*state)), GFP_NOFS);
> net/ceph/osdmap.c:      weight = kvmalloc(array_size(max, 
> sizeof(*weight)), GFP_NOFS);
> net/ceph/osdmap.c:      addr = kvmalloc(array_size(max, sizeof(*addr)), 
> GFP_NOFS);
> 
> and with GFP_NOIO flag too:
> 
> git grep kvmalloc.*NOIO
> drivers/md/dm-integrity.c:      recalc_tags = kvmalloc(recalc_tags_size, 
> GFP_NOIO);
> drivers/md/dm-ioctl.c:  dmi = kvmalloc(param_kernel->data_size, GFP_NOIO 
> | __GFP_HIGH);
> net/ceph/messenger_v2.c:        buf = kvmalloc(len, GFP_NOIO);
> net/ceph/osdmap.c:      work = kvmalloc(work_size, GFP_NOIO);
> 
> And AFAIU documentation https://docs.kernel.org/core-api/gfp_mask-from- 
> fs-io.html#what-about-vmalloc-gfp-nofs vmalloc allocation with GFP_NOFS 
> may end up doing "GFP_KERNEL allocations deep inside the allocator", 
> which can potentially lead to deadlock in IO/FS code paths.
> 
> Does it mean that we should rework all those paths to 
> memalloc_noio_save / memalloc_noio_restore variant? Or is it already 
> safe to use kvmalloc(GFP_NOIO) in modern kernel?
> 
> Or maybe I misunderstand something, sorry in advance if that's the case.
> 

Now when I've already sent a question I seemingly found the answer:

In commit 451769ebb7e79 ("mm/vmalloc: alloc GFP_NO{FS,IO} for vmalloc") 
we add implicit memalloc_noXX_save/memalloc_noXX_restore at this code path:

   +->kvmalloc
     +-> ...
       +-> __kvmalloc_node_noprof
         +-> __vmalloc_node_range_noprof
           +-> __vmalloc_area_node

So kvmalloc should be safe now with GFP_NOIO. Should we correct the 
documentation?

-- 
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.

Re: Question about vmalloc(GFP_NOFS)

[Michal Hocko]

On Tue 19-11-24 15:24:03, Pavel Tikhomirov wrote:
[...]
> In commit 451769ebb7e79 ("mm/vmalloc: alloc GFP_NO{FS,IO} for vmalloc") we
> add implicit memalloc_noXX_save/memalloc_noXX_restore at this code path:
> 
>   +->kvmalloc
>     +-> ...
>       +-> __kvmalloc_node_noprof
>         +-> __vmalloc_node_range_noprof
>           +-> __vmalloc_area_node
> 
> So kvmalloc should be safe now with GFP_NOIO.

Correct.

> Should we correct the documentation?

Yes, please. I think it would be useful to explicitly name the above
commit because pre 5.17 kernels or those who haven't backported it are
still in same position and that could get dangerous if they try to
backport [k]vmalloc GFP_NOFS patches. Thanks!

-- 
Michal Hocko
SUSE Labs

Re: Question about vmalloc(GFP_NOFS)

[Pavel Tikhomirov]

On 11/19/24 16:29, Michal Hocko wrote:
> On Tue 19-11-24 15:24:03, Pavel Tikhomirov wrote:
> [...]
>> In commit 451769ebb7e79 ("mm/vmalloc: alloc GFP_NO{FS,IO} for vmalloc") we
>> add implicit memalloc_noXX_save/memalloc_noXX_restore at this code path:
>>
>>    +->kvmalloc
>>      +-> ...
>>        +-> __kvmalloc_node_noprof
>>          +-> __vmalloc_node_range_noprof
>>            +-> __vmalloc_area_node
>>
>> So kvmalloc should be safe now with GFP_NOIO.
> 
> Correct.
> 
>> Should we correct the documentation?
> 
> Yes, please. I think it would be useful to explicitly name the above
> commit because pre 5.17 kernels or those who haven't backported it are
> still in same position and that could get dangerous if they try to
> backport [k]vmalloc GFP_NOFS patches. Thanks!
> 

Done 
https://lore.kernel.org/lkml/20241119093922.567138-1-ptikhomirov@virtuozzo.com/. 
Thank you for confirming!

-- 
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.