Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Tejun Heo <htejun@gmail.com>
Date:   Wed, 12 Apr 2023 08:27:39 -1000
From:   Tejun Heo <tj@kernel.org>
To:     Waiman Long <longman@redhat.com>
Cc:     Zefan Li <lizefan.x@bytedance.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Christian Brauner <brauner@kernel.org>,
        cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
        Juri Lelli <juri.lelli@redhat.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Michal =?iso-8859-1?Q?Koutn=FD?= <mkoutny@suse.com>,
        Giuseppe Scrivano <gscrivan@redhat.com>
Subject: Re: [PATCH v4 5/5] cgroup/cpuset: Optimize out unneeded
 cpuset_can_fork/cpuset_cancel_fork calls
Message-ID: <ZDb4G2jgQFK8h8Ys@slm.duckdns.org>
References: <20230411133601.2969636-1-longman@redhat.com>
 <20230411133601.2969636-6-longman@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230411133601.2969636-6-longman@redhat.com>
Precedence: bulk

On Tue, Apr 11, 2023 at 09:36:01AM -0400, Waiman Long wrote:
> The newly introduced cpuset_can_fork() and cpuset_cancel_fork() calls
> are only needed when the CLONE_INTO_CGROUP flag is set which is not
> likely. Adding an extra cpuset_can_fork() call does introduce a bit
> of performance overhead in the fork/clone fastpath. To reduce this
> performance overhead, introduce a new clone_into_cgroup_can_fork flag
> into the cgroup_subsys structure. This flag, when set, will call the
> can_fork and cancel_fork methods only if the CLONE_INTO_CGROUP flag
> is set.
> 
> The cpuset code is now modified to set this flag. The same cpuset
> checking code in cpuset_can_fork() and cpuset_cancel_fork() will have
> to stay as the cgroups can be different, but the cpusets may still be
> the same. So the same check must be present in both cpuset_fork() and
> cpuset_can_fork() to make sure that attach_in_progress is correctly set.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>

Waiman, I'm not necessarily against this optimization but can we at least
have some performance numbers to show that this is actually meaningful?
Given how heavy our fork path is, I'm not too sure this would show up in any
meaningful way.

Thanks.

-- 
tejun