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: <e7d6c19b-593d-acfd-35af-73b1840be276@arm.com>
Date:   Tue, 6 Dec 2022 18:54:39 +0100
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 v2 3/7] sched: Teach arch_asym_cpu_priority() the idle
 state of SMT siblings
Content-Language: en-US
To:     Ricardo Neri <ricardo.neri-calderon@linux.intel.com>,
        "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        Juri Lelli <juri.lelli@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>
Cc:     Ricardo Neri <ricardo.neri@intel.com>,
        "Ravi V. Shankar" <ravi.v.shankar@intel.com>,
        Ben Segall <bsegall@google.com>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Len Brown <len.brown@intel.com>, Mel Gorman <mgorman@suse.de>,
        "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        Valentin Schneider <vschneid@redhat.com>, x86@kernel.org,
        linux-kernel@vger.kernel.org, "Tim C . Chen" <tim.c.chen@intel.com>
References: <20221122203532.15013-1-ricardo.neri-calderon@linux.intel.com>
 <20221122203532.15013-4-ricardo.neri-calderon@linux.intel.com>
From:   Dietmar Eggemann <dietmar.eggemann@arm.com>
In-Reply-To: <20221122203532.15013-4-ricardo.neri-calderon@linux.intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 22/11/2022 21:35, Ricardo Neri wrote:
> Some processors (e.g., Intel processors with ITMT) use asym_packing to
> balance load between physical cores with SMT. In such case, a core with all
> its SMT siblings idle is more desirable than another with one or more busy
> siblings.
> 
> Other processors (e.g, Power7 with SMT8) use asym_packing to balance load
> among SMT siblings of different priority, regardless of their idle state.
> 
> Add a new parameter, check_smt, that architectures can use as needed.

[...]

> Changes since v1:
>  * Introduced this patch.

[...]

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index d18947a9c03e..0e4251f83807 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -142,8 +142,11 @@ __setup("sched_thermal_decay_shift=", setup_sched_thermal_decay_shift);
>  #ifdef CONFIG_SMP
>  /*
>   * For asym packing, by default the lower numbered CPU has higher priority.
> + *
> + * When doing ASYM_PACKING at the "MC" or higher domains, architectures may

There is this new CLUSTER level between SMT and MC.

> + * want to check the idle state of the SMT siblngs of @cpu.

s/siblngs/siblings

The scheduler calls sched_asym_prefer(..., true) from
find_busiest_queue(), asym_active_balance() and nohz_balancer_kick()
even from SMT layer on !x86. So I guess a `bool check_smt` wouldn't be
sufficient to distinguish whether sched_smt_siblings_idle() should be
called or not. To me this comment is a little bit misleading. Not an
issue currently since there is only the x86 overwrite right now.

>   */
> -int __weak arch_asym_cpu_priority(int cpu)
> +int __weak arch_asym_cpu_priority(int cpu, bool check_smt)
>  {
>  	return -cpu;
>  }

[...]