On 2019/9/23 11:50, Nicolas Pitre wrote:
> On Sat, 21 Sep 2019, Xiaoming Ni wrote:
>
>> @ Nicolas Pitre
>> Can I make a v2 patch based on your advice ?
>> Or you will submit a patch for "GFP_WONTFAIL" yourself ?
>
> Here's a patch implementing what I had in mind. This is compile tested
> only.
>
> ----- >8
>
> Subject: [PATCH] mm: add __GFP_WONTFAIL and GFP_ONBOOT
>
> Some memory allocations are very unlikely to fail during system boot.
> Because of that, the code often doesn't bother to check for allocation
> failure, but this gets reported anyway.
>
> As an alternative to adding code to check for NULL that has almost no
> chance of ever being exercised, let's use a GFP flag to identify those
> cases and panic the kernel if allocation failure ever occurs.
>
> Conversion of one such instance is also included.
>
> Signed-off-by: Nicolas Pitre <n...@fluxnic.net>
>
.....
....
> /**
> @@ -285,6 +293,9 @@ struct vm_area_struct;
> * available and will not wake kswapd/kcompactd on failure. The _LIGHT
> * version does not attempt reclaim/compaction at all and is by default used
> * in page fault path, while the non-light is used by khugepaged.
> + *
> + * %GFP_ONBOOT is for relatively small allocations that are not expected
> + * to fail while the system is booting.
> */
> #define GFP_ATOMIC (__GFP_HIGH|__GFP_ATOMIC|__GFP_KSWAPD_RECLAIM)
> #define GFP_KERNEL (__GFP_RECLAIM | __GFP_IO | __GFP_FS)
> @@ -300,6 +311,7 @@ struct vm_area_struct;
> #define GFP_TRANSHUGE_LIGHT ((GFP_HIGHUSER_MOVABLE | __GFP_COMP | \
> __GFP_NOMEMALLOC | __GFP_NOWARN) & ~__GFP_RECLAIM)
> #define GFP_TRANSHUGE (GFP_TRANSHUGE_LIGHT | __GFP_DIRECT_RECLAIM)
> +#define GFP_ONBOOT (GFP_NOWAIT | __GFP_WONTFAIL)
>
Isn't it better to bind GFP_ONBOOT and GFP_NOWAIT?
Can be not GFP_NOWAIT when applying for memory at boot time
> /* Convert GFP flags to their corresponding migrate type */
> #define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE)
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index ff5484fdbd..36dee09f7f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4625,6 +4625,14 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int
> order,
> fail:
> warn_alloc(gfp_mask, ac->nodemask,
> "page allocation failure: order:%u", order);
> + if (gfp_mask & __GFP_WONTFAIL) {
Is it more intuitive to use __GFP_DIE_IF_FAIL as the flag name?
> + /*
> + * The assumption was wrong. This is never supposed to happen.
> + * Caller most likely won't check for a returned NULL either.
> + * So the only reasonable thing to do is to pannic.
> + */
> + panic("Failed to allocate memory despite GFP_WONTFAIL\n");
> + }
> got_pg:
> return page;
> }
>
> .
>
thanks
Niaoming Ni
