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;
Message-ID: <92f68392-12d2-f64a-9bb9-1a3a15f99d02@redhat.com>
Date:   Fri, 29 Jul 2022 10:50:30 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH 1/2] cgroup/cpuset: Keep current cpus list if cpus
 affinity was explicitly set
Content-Language: en-US
To:     Valentin Schneider <vschneid@redhat.com>, Tejun Heo <tj@kernel.org>
Cc:     Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Juri Lelli <juri.lelli@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Zefan Li <lizefan.x@bytedance.com>,
        Johannes Weiner <hannes@cmpxchg.org>, cgroups@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220728005815.1715522-1-longman@redhat.com>
 <YuLF+xXaCzwWi2BR@slm.duckdns.org>
 <1ae1cc6c-dca9-4958-6b22-24a5777c5e8d@redhat.com>
 <YuLdX7BYGvo57LNU@slm.duckdns.org>
 <606ed69e-8ad0-45d5-9de7-48739df7f48d@redhat.com>
 <YuL1NijxSEv2xadk@slm.duckdns.org>
 <c470d3f7-f0f8-b8e6-4a95-7b334f0a824b@redhat.com>
 <YuMCB86fH2K3NcqM@slm.duckdns.org> <xhsmhy1wcc8dq.mognet@vschneid.remote.csb>
From:   Waiman Long <longman@redhat.com>
In-Reply-To: <xhsmhy1wcc8dq.mognet@vschneid.remote.csb>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/29/22 10:15, Valentin Schneider wrote:
> On 28/07/22 11:39, Tejun Heo wrote:
>> Hello, Waiman.
>>
>> On Thu, Jul 28, 2022 at 05:04:19PM -0400, Waiman Long wrote:
>>>> So, the patch you proposed is making the code remember one special aspect of
>>>> user requested configuration - whether it configured it or not, and trying
>>>> to preserve that particular state as cpuset state changes. It addresses the
>>>> immediate problem but it is a very partial approach. Let's say a task wanna
>>>> be affined to one logical thread of each core and set its mask to 0x5555.
>>>> Now, let's say cpuset got enabled and enforced 0xff and affined the task to
>>>> 0xff. After a while, the cgroup got more cpus allocated and its cpuset now
>>>> has 0xfff. Ideally, what should happen is the task now having the effective
>>>> mask of 0x555. In practice, tho, it either would get 0xf55 or 0x55 depending
>>>> on which way we decide to misbehave.
>>> OK, I see what you want to accomplish. To fully address this issue, we will
>>> need to have a new cpumask variable in the the task structure which will be
>>> allocated if sched_setaffinity() is ever called. I can rework my patch to
>>> use this approach.
>> Yeah, we'd need to track what user requested separately from the currently
>> effective cpumask. Let's make sure that the scheduler folks are on board
>> before committing to the idea tho. Peter, Ingo, what do you guys think?
>>
> FWIW on a runtime overhead side of things I think it'll be OK as that
> should be just an extra mask copy  in sched_setaffinity() and a subset
> check / cpumask_and() in set_cpus_allowed_ptr(). The policy side is a bit
> less clear (when, if ever, do we clear the user-defined mask? Will it keep
> haunting us even after moving a task to a disjoint cpuset partition?).

The runtime overhead should be minimal. It is the behavioral side that 
we should be careful about. It is a change in existing behavior and we 
don't want to cause surprise to the users. Currently, a task that set 
its cpu affinity explicitly will have its affinity reset whenever there 
is any change to the cpuset it belongs to or a hotplug event touch any 
cpu in the current cpuset. The new behavior we are proposing here is 
that it will try its best to keep the cpu affinity that the user 
requested within the constraint of the current cpuset as well as the cpu 
hotplug state.


>
> There's also if/how that new mask should be exposed, because attaching a
> task to a cpuset will now yield a not-necessarily-obvious affinity -
> e.g. in the thread affinity example above, if the initial affinity setting
> was done ages ago by some system tool, IMO the user needs a way to be able
> to expect/understand the result of 0x555 rather than 0xfff.

Users can use sched_getaffinity(2) to retrieve the current cpu affinity. 
It is up to users to set another one if they don't like the current one. 
I don't think we need to return what the previous requested cpu affinity 
is. They are suppose to know that or they can set their own if they 
don't like it. \

Cheers,
Longman

>
> While I'm saying this, I don't think anything exposes p->user_cpus_ptr, but
> then again that one is for "special" hardware...
>
>> Thanks.
>>
>> --
>> tejun