Received-SPF: pass (google.com: domain of linux-kernel+bounces-207594-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH -next] cgroup: Fix AA deadlock caused by cgroup_bpf_release
From: Roman Gushchin <roman.gushchin@linux.dev>
In-Reply-To: <20240607110313.2230669-1-chenridong@huawei.com>
Date: Sun, 9 Jun 2024 19:47:55 -0700
Cc: martin.lau@linux.dev, ast@kernel.org, daniel@iogearbox.net,
 andrii@kernel.org, eddyz87@gmail.com, song@kernel.org, yonghong.song@linux.dev,
 john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
 haoluo@google.com, jolsa@kernel.org, tj@kernel.org, lizefan.x@bytedance.com,
 hannes@cmpxchg.org, bpf@vger.kernel.org, cgroups@vger.kernel.org,
 linux-kernel@vger.kernel.org
Message-Id: <67B5A5C8-68D8-499E-AFF1-4AFE63128706@linux.dev>
References: <20240607110313.2230669-1-chenridong@huawei.com>
To: Chen Ridong <chenridong@huawei.com>

Hi Chen!

Was this problem found in the real life? Do you have a LOCKDEP splash availa=
ble?

> On Jun 7, 2024, at 4:09=E2=80=AFAM, Chen Ridong <chenridong@huawei.com> wr=
ote:
>=20
> =EF=BB=BFWe found an AA deadlock problem as shown belowed:
>=20
> cgroup_destroy_wq        TaskB                WatchDog            system_w=
q
>=20
> ...
> css_killed_work_fn:
> P(cgroup_mutex)
> ...
>                                ...
>                                __lockup_detector_reconfigure:
>                                P(cpu_hotplug_lock.read)
>                                ...
>                ...
>                percpu_down_write:
>                P(cpu_hotplug_lock.write)
>                                                ...
>                                                cgroup_bpf_release:
>                                                P(cgroup_mutex)
>                                smp_call_on_cpu:
>                                Wait system_wq
>=20
> cpuset_css_offline:
> P(cpu_hotplug_lock.read)
>=20
> WatchDog is waiting for system_wq, who is waiting for cgroup_mutex, to
> finish the jobs, but the owner of the cgroup_mutex is waiting for
> cpu_hotplug_lock. This problem caused by commit 4bfc0bb2c60e ("bpf:
> decouple the lifetime of cgroup_bpf from cgroup itself")
> puts cgroup_bpf release work into system_wq. As cgroup_bpf is a member of
> cgroup, it is reasonable to put cgroup bpf release work into
> cgroup_destroy_wq, which is only used for cgroup's release work, and the
> preblem is solved.

I need to think more on this, but at first glance the fix looks a bit confus=
ing. cgroup_bpf_release() looks quite innocent, it only takes a cgroup_mutex=
. It=E2=80=99s not obvious why it=E2=80=99s not ok and requires a dedicated w=
ork queue. What exactly is achieved by placing it back on the dedicated cgro=
up destroy queue?

I=E2=80=99m not trying to say your fix won=E2=80=99t work, but it looks like=
 it might cover a more serious problem.=