Re: cgroups: warning for metadata allocation with GFP_NOFAIL (was Re: folio_alloc_buffers() doing allocations > order 1 with GFP_NOFAIL)

[Shakeel Butt <shakeelb@google.com>]

+Andrew

On Tue, Nov 7, 2023 at 10:05 AM Roman Gushchin <roman.gushchin@linux.dev> wrote:
>
> On Mon, Nov 06, 2023 at 06:57:05PM -0800, Christoph Lameter wrote:
> > Right.. Well lets add the cgoup folks to this.
>
> Hello!
>
> I think it's the best thing we can do now. Thoughts?
>
> From 5ed3e88f4f052b6ce8dbec0545dfc80eb7534a1a Mon Sep 17 00:00:00 2001
> From: Roman Gushchin <roman.gushchin@linux.dev>
> Date: Tue, 7 Nov 2023 09:18:02 -0800
> Subject: [PATCH] mm: kmem: drop __GFP_NOFAIL when allocating objcg vectors
>
> Objcg vectors attached to slab pages to store slab object ownership
> information are allocated using gfp flags for the original slab
> allocation. Depending on slab page order and the size of slab objects,
> objcg vector can take several pages.
>
> If the original allocation was done with the __GFP_NOFAIL flag, it
> triggered a warning in the page allocation code. Indeed, order > 1
> pages should not been allocated with the __GFP_NOFAIL flag.
>
> Fix this by simple dropping the __GFP_NOFAIL flag when allocating

*simply

> the objcg vector. It effectively allows to skip the accounting of a
> single slab object under a heavy memory pressure.
>
> An alternative would be to implement the mechanism to fallback to
> order-0 allocations for accounting metadata, which is also not perfect
> because it will increase performance penalty and memory footprint
> of the kernel memory accounting under memory pressure.
>
> Reported-by: Christoph Lameter <cl@linux.com>
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>
> Cc: Matthew Wilcox <willy@infradead.org>

I think we should CC stable too.

Acked-by: Shakeel Butt <shakeelb@google.com>


> ---
>  mm/memcontrol.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 774bd6e21e27..1c1061df9cd1 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2936,7 +2936,8 @@ void mem_cgroup_commit_charge(struct folio *folio, struct mem_cgroup *memcg)
>   * Moreover, it should not come from DMA buffer and is not readily
>   * reclaimable. So those GFP bits should be masked off.
>   */
> -#define OBJCGS_CLEAR_MASK      (__GFP_DMA | __GFP_RECLAIMABLE | __GFP_ACCOUNT)
> +#define OBJCGS_CLEAR_MASK      (__GFP_DMA | __GFP_RECLAIMABLE | \
> +                                __GFP_ACCOUNT | __GFP_NOFAIL)
>
>  /*
>   * mod_objcg_mlstate() may be called with irq enabled, so
> --
> 2.42.0
>