Re: [PATCH v2 5/7] x86/sched: Remove SD_ASYM_PACKING from the "SMT" domain

From: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
To: Valentin Schneider <vschneid@redhat.com>
Cc: Ionela Voinescu <ionela.voinescu@arm.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	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>,
	Dietmar Eggemann <dietmar.eggemann@arm.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>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	"Tim C . Chen" <tim.c.chen@intel.com>
Subject: Re: [PATCH v2 5/7] x86/sched: Remove SD_ASYM_PACKING from the "SMT" domain
Date: Thu, 29 Dec 2022 11:02:49 -0800	[thread overview]
Message-ID: <20221229190249.GA14462@ranerica-svr.sc.intel.com> (raw)
In-Reply-To: <xhsmhr0wre5rg.mognet@vschneid.remote.csb>

On Thu, Dec 22, 2022 at 04:56:51PM +0000, Valentin Schneider wrote:
> On 19/12/22 16:42, Ricardo Neri wrote:
> > On Thu, Dec 15, 2022 at 04:48:14PM +0000, Valentin Schneider wrote:
> >> With that said, so far all but one flag (SD_PREFER_SIBLING, and that's
> >> because of big.LITTLE woes) follow the SDF_SHARED_{CHILD, PARENT} pattern,
> >> if SD_ASYM_PACKING no longer does then we need to think whether we're
> >> trying to make it do funky things.
> >
> > My thesis is that x86 does not need the SD_ASYM_PACKING flag at the SMT
> > level because all SMT siblings are identical. There are cores of higher
> > priority at the "MC" level (maybe in the future at the "CLS" level).
> >
> > Power7 is fine because it only uses SD_ASYM_PACKING at the SMT level.
> >
> 
> So with what I groked from your series, I agree with you, x86 shouldn't
> need it at SMT level.
> 
> What about the below?
> 
> ---
> 
> diff --git a/include/linux/sched/sd_flags.h b/include/linux/sched/sd_flags.h
> index 57bde66d95f7a..8dc16942135b4 100644
> --- a/include/linux/sched/sd_flags.h
> +++ b/include/linux/sched/sd_flags.h
> @@ -132,12 +132,12 @@ SD_FLAG(SD_SERIALIZE, SDF_SHARED_PARENT | SDF_NEEDS_GROUPS)
>  /*
>   * Place busy tasks earlier in the domain
>   *
> - * SHARED_CHILD: Usually set on the SMT level. Technically could be set further
> - *               up, but currently assumed to be set from the base domain
> - *               upwards (see update_top_cache_domain()).
> + * SHARED_PARENT: Usually set on the SMT level. Can be set further up if all
> + *                siblings of an SMT core are identical, but SMT cores themselves
> + *                have different priorites.
>   * NEEDS_GROUPS: Load balancing flag.
>   */
> -SD_FLAG(SD_ASYM_PACKING, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
> +SD_FLAG(SD_ASYM_PACKING, SDF_SHARED_PARENT | SDF_NEEDS_GROUPS)

But this would not work for Power7. It only has SD_ASYM_PACKING in the SMT
sched domain. Must it have either of these flags?

In Power7 SMT siblings have the different priority but, IIUC, physical
cores are identical.

It seems to me that asym_packing is specific to a domain.

Thanks and BR,
Ricardo