Re: [RFC PATCH 3/3] cgroup: Do not take css_set_lock in cgroup_show_path

From: Tejun Heo <tj@kernel.org>
To: "Michal Koutný" <mkoutny@suse.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	cgroups@vger.kernel.org, Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	Zefan Li <lizefan.x@bytedance.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Dave Chinner <dchinner@redhat.com>,
	Rik van Riel <riel@surriel.com>, Jiri Wiesner <jwiesner@suse.de>
Subject: Re: [RFC PATCH 3/3] cgroup: Do not take css_set_lock in cgroup_show_path
Date: Fri, 5 May 2023 05:45:58 -1000	[thread overview]
Message-ID: <ZFUktg4Yxa30jRBX@slm.duckdns.org> (raw)
In-Reply-To: <20230502133847.14570-4-mkoutny@suse.com>

Hello,

On Tue, May 02, 2023 at 03:38:47PM +0200, Michal Koutný wrote:
> /proc/$pid/mountinfo may accumulate lots of entries (causing frequent
> re-reads of whole file) or lots cgroupfs entries alone.
> The cgroupfs entries rendered with cgroup_show_path() may increase/be
> subject of css_set_lock contention causing further slowdown -- not only
> mountinfo rendering but any other css_set_lock user.
> 
> We leverage the fact that mountinfo reading happens with namespace_sem
> taken and hierarchy roots thus cannot be freed concurrently.
> 
> There are three relevant nodes for each cgroupfs entry:
> 
>         R ... cgroup hierarchy root
>         M ... mount root
>         C ... reader's cgroup NS root
> 
> mountinfo is supposed to show path from C to M.

At least for cgroup2, the path from C to M isn't gonna change once NS is
established, right? Nothing can be moved or renamed while the NS root is
there. If so, can't we just cache the formatted path and return the same
thing without any locking? The proposed changes seem a bit too brittle to
me.

Thanks.

-- 
tejun