Re: [PATCH] memcg: Remove memcg_cgroup::id from IDR on mem_cgroup_css_alloc() failure

On Mon, Jul 30, 2018 at 11:31:13AM -0400, Johannes Weiner wrote:

On Sun, Jul 29, 2018 at 10:26:21PM +0300, Vladimir Davydov wrote:

quoted

On Fri, Jul 27, 2018 at 03:31:34PM -0400, Johannes Weiner wrote:

quoted

That said, the lifetime of the root reference on the ID is the online
state, we put that in css_offline. Is there a reason we need to have
the ID ready and the memcg in the IDR before onlining it?

I fail to see any reason for this in the code.

Me neither, thanks for double checking.

The patch also survives stress testing cgroup creation and destruction
with the script from 73f576c04b94 ("mm: memcontrol: fix cgroup
creation failure after many small jobs").

quoted

quoted

Can we do something like this and not mess with the alloc/free
sequence at all?

I guess so, and this definitely looks better to me.

Cool, then I think we should merge Kirill's patch as the fix and mine
as a follow-up cleanup.

---

From b4106ea1f163479da805eceada60c942bd66e524 Mon Sep 17 00:00:00 2001
From: Johannes Weiner <hannes@cmpxchg.org>
Date: Mon, 30 Jul 2018 11:03:55 -0400
Subject: [PATCH] mm: memcontrol: simplify memcg idr allocation and error
 unwinding

The memcg ID is allocated early in the multi-step memcg creation
process, which needs 2-step ID allocation and IDR publishing, as well
as two separate IDR cleanup/unwind sites on error.

Defer the IDR allocation until the last second during onlining to
eliminate all this complexity. There is no requirement to have the ID
and IDR entry earlier than that. And the root reference to the ID is
put in the offline path, so this matches nicely.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Acked-by: Vladimir Davydov <redacted>