On Thu, May 1, 2025 at 5:18 PM Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote:
>
> There is no need to disable irqs to use memcg per-cpu stock, so let's
> just not do that. One consequence of this change is if the kernel while
> in task context has the memcg stock lock and that cpu got interrupted.
> The memcg charges on that cpu in the irq context will take the slow path
> of memcg charging. However that should be super rare and should be fine
> in general.
>
> Signed-off-by: Shakeel Butt <shakeel.butt@xxxxxxxxx>
> Acked-by: Vlastimil Babka <vbabka@xxxxxxx>
> ---
> mm/memcontrol.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index cd81c70d144b..f8b9c7aa6771 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1858,7 +1858,6 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages,
> {
> struct memcg_stock_pcp *stock;
> uint8_t stock_pages;
> - unsigned long flags;
> bool ret = false;
> int i;
>
> @@ -1866,8 +1865,8 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages,
> return ret;
>
> if (gfpflags_allow_spinning(gfp_mask))
> - local_lock_irqsave(&memcg_stock.lock, flags);
> - else if (!local_trylock_irqsave(&memcg_stock.lock, flags))
> + local_lock(&memcg_stock.lock);
> + else if (!local_trylock(&memcg_stock.lock))
> return ret;
I don't think it works.
When there is a normal irq and something doing regular GFP_NOWAIT
allocation gfpflags_allow_spinning() will be true and
local_lock() will reenter and complain that lock->acquired is
already set... but only with lockdep on.
