lkml.org 
[lkml]   [2023]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH] mm/page_alloc: avoid high-order page allocation warn with __GFP_NOFAIL
From


On 2023/3/6 15:51, Michal Hocko wrote:
> [Cc couple of more people recently involved with vmalloc code]
>
> On Sun 05-03-23 13:30:35, Gao Xiang wrote:
>> My knowledge of this is somewhat limited, however, since vmalloc already
>> supported __GFP_NOFAIL in commit 9376130c390a ("mm/vmalloc: add
>> support for __GFP_NOFAIL"). __GFP_NOFAIL could trigger the following
>> stack and allocate high-order pages when CONFIG_HAVE_ARCH_HUGE_VMALLOC
>> is enabled:
>>
>> __alloc_pages+0x1cb/0x5b0 mm/page_alloc.c:5549
>> alloc_pages+0x1aa/0x270 mm/mempolicy.c:2286
>> vm_area_alloc_pages mm/vmalloc.c:2989 [inline]
>>
>> __vmalloc_area_node mm/vmalloc.c:3057 [inline]
>> __vmalloc_node_range+0x978/0x13c0 mm/vmalloc.c:3227
>> kvmalloc_node+0x156/0x1a0 mm/util.c:606
>> kvmalloc include/linux/slab.h:737 [inline]
>> kvmalloc_array include/linux/slab.h:755 [inline]
>> kvcalloc include/linux/slab.h:760 [inline]
>> (codebase: Linux 6.2-rc2)
>>
>> Don't warn such cases since high-order pages with __GFP_NOFAIL is
>> somewhat legel.
>
> OK, this is definitely a bug and it seems my 9376130c390a was
> incomplete because it hasn't covered the high order case. Not sure how
> that happened but removing the warning is not the right thing to do
> here. The higher order allocation is an optimization rather than a must.
> So it is perfectly fine to fail that allocation and retry rather than
> go into a very expensive and potentially impossible higher order
> allocation that must not fail.
>
> The proper fix should look like this unless I am missing something. I
> would appreciate another pair of eyes on this because I am not fully
> familiar with the high order optimization part much.

I'm fine with the fix. Although I'm not familiar with such vmalloc
allocation, I thought about this possibility as well.

The original issue was:
https://lore.kernel.org/r/0000000000007796bd05f1852ec2@google.com

which I used kvcalloc with __GFP_NOFAIL but it warned, and I made
a fix (which now seems wrong) to use kcalloc() but it now warns
the same:
https://lore.kernel.org/r/00000000000072eb6505f376dd4b@google.com

And I then realized it's a bug in kvmalloc() with __GFP_NOFAIL...

Thanks,
Gao Xiang

>
> Thanks!
> ---
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index ef910bf349e1..a8aa2765618a 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2883,6 +2883,8 @@ vm_area_alloc_pages(gfp_t gfp, int nid,
> unsigned int order, unsigned int nr_pages, struct page **pages)
> {
> unsigned int nr_allocated = 0;
> + gfp_t alloc_gfp = gfp;
> + bool nofail = false;
> struct page *page;
> int i;
>
> @@ -2931,20 +2933,30 @@ vm_area_alloc_pages(gfp_t gfp, int nid,
> if (nr != nr_pages_request)
> break;
> }
> + } else {
> + alloc_gfp &= ~__GFP_NOFAIL;
> + nofail = true;
> }
>
> /* High-order pages or fallback path if "bulk" fails. */
> -
> while (nr_allocated < nr_pages) {
> if (fatal_signal_pending(current))
> break;
>
> if (nid == NUMA_NO_NODE)
> - page = alloc_pages(gfp, order);
> + page = alloc_pages(alloc_gfp, order);
> else
> - page = alloc_pages_node(nid, gfp, order);
> - if (unlikely(!page))
> - break;
> + page = alloc_pages_node(nid, alloc_gfp, order);
> + if (unlikely(!page)) {
> + if (!nofail)
> + break;
> +
> + /* fall back to the zero order allocations */
> + alloc_gfp |= __GFP_NOFAIL;
> + order = 0;
> + continue;
> + }
> +
> /*
> * Higher order allocations must be able to be treated as
> * indepdenent small pages by callers (as they can with

\
 
 \ /
  Last update: 2023-03-27 00:44    [W:0.110 / U:0.188 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site