Re: [PATCH v2 3/5] mm/mglru: cache the last memcg node, make tail insertion constant time

[Barry Song]

On Tue, Nov 3, 2026 at 7:37 PM Max Kellermann <max.kellermann@xxxxxxxxx> wrote:
>
> Rotating or onlining a memcg currently uses hlist_nulls_add_tail_rcu()
> which walks the destination bin to find its tail while holding
> `memcg_lru.lock` with interrupts disabled.  With thousands of cgroups,
> this takes a long time and causes severe lock contention.

This is the main cause of the soft lockups and RCU stalls,
right? If so, we may want to add a Fixes tag.

The fix itself may not be directly applicable to stable
kernels, but based on the logic of this patch, we should
be able to work out a backport, right?

Another option is to leave the cleanups in patches 1 and 2
out for now, and make patch 3 self-contained as the first
patch in your series, if that makes backporting easier.

You can then move the cleanups from patches 1 and 2 to
follow the new patch 1.

>
> Cache a pointer to each bin's last node, making tail insertion O(1).
> This requires remembering the bin number (but the new struct field
> occupies a hole, so it does not grow).
>
> In a micro benchmark that creates 1500 cgroups and repeatedly writes
> `1024M swappiness=0` to `memory.reclaim` in 8 threads, the request
> rate increases by a factor of 9.5 and the average duration of each
> write request is reduced by 89%.
>
> All writes return EAGAIN in both runs; the benchmark measures the cost
> of reclaim attempts that cannot meet their requested target, not
> reclaimed throughput.

I'd appreciate it if you could also mention Ridong's earlier
work in the changelog:

https://lore.kernel.org/all/20251119083722.1365680-1-chenridong@xxxxxxxxxxxxxxx/

>
> Signed-off-by: Max Kellermann <max.kellermann@xxxxxxxxx>
> ---
[...]
> @@ -4561,21 +4561,37 @@ static void lru_gen_add_memcg(struct lru_gen_memcg *memcg_lru,
>                               int gen, int bin, bool at_head)
>  {
>         struct hlist_nulls_head *head = &memcg_lru->fifo[gen][bin];
> +       struct hlist_nulls_node *last = memcg_lru->tail[gen][bin];
>
>         lockdep_assert_held(&memcg_lru->lock);
>
> -       if (at_head)
> +       lrugen->bin = bin;
> +
> +       if (at_head || !last)
>                 hlist_nulls_add_head_rcu(&lrugen->list, head);

Do we still need at_head now that MEMCG_LRU_OLD has been removed?

>         else
> -               hlist_nulls_add_tail_rcu(&lrugen->list, head);
> +               hlist_nulls_add_behind_rcu(&lrugen->list, last);
> +
> +       if (!at_head || !last)
> +               memcg_lru->tail[gen][bin] = &lrugen->list;
>  }

Best,
Barry