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: <b7ad39b1-c615-3ebc-6980-d9db0f2ab0a0@redhat.com>
Date:   Wed, 29 Mar 2023 14:09:42 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH 5/6] cgroup/cpuset: Free DL BW in case can_attach() fails
Content-Language: en-US
To:     Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Juri Lelli <juri.lelli@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        Qais Yousef <qyousef@layalina.io>, Tejun Heo <tj@kernel.org>,
        Zefan Li <lizefan.x@bytedance.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Hao Luo <haoluo@google.com>
Cc:     Steven Rostedt <rostedt@goodmis.org>, linux-kernel@vger.kernel.org,
        luca.abeni@santannapisa.it, claudio@evidence.eu.com,
        tommaso.cucinotta@santannapisa.it, bristot@redhat.com,
        mathieu.poirier@linaro.org, cgroups@vger.kernel.org,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Wei Wang <wvw@google.com>, Rick Yiu <rickyiu@google.com>,
        Quentin Perret <qperret@google.com>,
        Heiko Carstens <hca@linux.ibm.com>,
        Vasily Gorbik <gor@linux.ibm.com>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        Sudeep Holla <sudeep.holla@arm.com>
References: <20230329125558.255239-1-juri.lelli@redhat.com>
 <20230329125558.255239-6-juri.lelli@redhat.com>
 <f8dfc30b-5079-2f44-7ab1-42ac25bd48b7@redhat.com>
 <f8baea06-eeda-439a-3699-1cad7cde659e@redhat.com>
 <cdede77a-5dc5-8933-a444-a2046b074b12@arm.com>
From:   Waiman Long <longman@redhat.com>
In-Reply-To: <cdede77a-5dc5-8933-a444-a2046b074b12@arm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 3/29/23 12:39, Dietmar Eggemann wrote:
> On 29/03/2023 16:31, Waiman Long wrote:
>> On 3/29/23 10:25, Waiman Long wrote:
>>> On 3/29/23 08:55, Juri Lelli wrote:
>>>> From: Dietmar Eggemann <dietmar.eggemann@arm.com>
> [...]
>
>>>> @@ -2518,11 +2547,21 @@ static int cpuset_can_attach(struct
>>>> cgroup_taskset *tset)
>>>>    static void cpuset_cancel_attach(struct cgroup_taskset *tset)
>>>>    {
>>>>        struct cgroup_subsys_state *css;
>>>> +    struct cpuset *cs;
>>>>          cgroup_taskset_first(tset, &css);
>>>> +    cs = css_cs(css);
>>>>          mutex_lock(&cpuset_mutex);
>>>> -    css_cs(css)->attach_in_progress--;
>>>> +    cs->attach_in_progress--;
>>>> +
>>>> +    if (cs->nr_migrate_dl_tasks) {
>>>> +        int cpu = cpumask_any(cs->effective_cpus);
>>>> +
>>>> +        dl_bw_free(cpu, cs->sum_migrate_dl_bw);
>>>> +        reset_migrate_dl_data(cs);
>>>> +    }
>>>> +
>> Another nit that I have is that you may have to record also the cpu
>> where the DL bandwidth is allocated in cpuset_can_attach() and free the
>> bandwidth back into that cpu or there can be an underflow if another cpu
>> is chosen.
> Many thanks for the review!
>
> But isn't the DL BW control `struct dl_bw` per `struct root_domain`
> which is per exclusive cpuset. So as long cpu is from
> `cs->effective_cpus` shouldn't this be fine?

Sorry for my ignorance on how the deadline bandwidth operation work. I 
check the bandwidth code and find that we are storing the bandwidth 
information in the root domain, not on the cpu. That shouldn't be a 
concern then.

However, I still have some question on how that works when dealing with 
cpuset. First of all, not all the CPUs in a given root domains are in 
the cpuset. So there may be enough bandwidth on the root domain, but it 
doesn't mean there will be enough bandwidth in the set of CPUs in a 
particular cpuset. Secondly, how do you deal with isolated CPUs that do 
not have a corresponding root domain? It is now possible to create a 
cpuset with isolated CPUs.

Cheers,
Longman