From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 17E0EC6FD1D for ; Thu, 30 Mar 2023 15:16:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232820AbjC3PQ5 (ORCPT ); Thu, 30 Mar 2023 11:16:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33556 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232802AbjC3PQy (ORCPT ); Thu, 30 Mar 2023 11:16:54 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id A7E29C176; Thu, 30 Mar 2023 08:15:27 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B32732F4; Thu, 30 Mar 2023 08:14:59 -0700 (PDT) Received: from [192.168.178.6] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E1F6A3F6C4; Thu, 30 Mar 2023 08:14:11 -0700 (PDT) Message-ID: <5ff103f9-1366-0a9b-bd97-419ced1de07f@arm.com> Date: Thu, 30 Mar 2023 17:14:06 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH 5/6] cgroup/cpuset: Free DL BW in case can_attach() fails Content-Language: en-US To: Waiman Long , Juri Lelli , Peter Zijlstra , Ingo Molnar , Qais Yousef , Tejun Heo , Zefan Li , Johannes Weiner , Hao Luo Cc: Steven Rostedt , 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 , Wei Wang , Rick Yiu , Quentin Perret , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Sudeep Holla References: <20230329125558.255239-1-juri.lelli@redhat.com> <20230329125558.255239-6-juri.lelli@redhat.com> From: Dietmar Eggemann In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 29/03/2023 20:09, Waiman Long wrote: > 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 >> [...] >> >>>>> @@ -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. Sorry, I overlooked this email somehow. IMHO, this is only done for exclusive cpusets: cpuset_can_attach() if (!cpumask_intersects(oldcs->effective_cpus, cs->effective_cpus)) So they should have their own root_domain congruent to their cpumask.