Messages in this thread Patch in this message | | | Date | Wed, 14 Dec 2022 08:59:00 -0800 | From | Ricardo Neri <> | Subject | Re: [PATCH v2 5/7] x86/sched: Remove SD_ASYM_PACKING from the "SMT" domain |
| |
On Thu, Dec 08, 2022 at 04:03:04PM +0000, Ionela Voinescu wrote: > Hi Ricardo,
Hi Ionela,
Thank you very much for your feedback!
> > On Tuesday 22 Nov 2022 at 12:35:30 (-0800), Ricardo Neri wrote: > > There is no difference between any of the SMT siblings of a physical core. > > asym_packing load balancing is not needed among siblings. > > > > When balancing load among physical cores, the scheduler now considers the > > state of the siblings when checking the priority of a CPU. > > > > Cc: Ben Segall <bsegall@google.com> > > Cc: Daniel Bristot de Oliveira <bristot@redhat.com> > > Cc: Dietmar Eggemann <dietmar.eggemann@arm.com> > > Cc: Len Brown <len.brown@intel.com> > > Cc: Mel Gorman <mgorman@suse.de> > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > > Cc: Steven Rostedt <rostedt@goodmis.org> > > Cc: Tim C. Chen <tim.c.chen@intel.com> > > Cc: Valentin Schneider <vschneid@redhat.com> > > Cc: x86@kernel.org > > Cc: linux-kernel@vger.kernel.org > > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > > --- > > Changes since v1: > > * Introduced this patch. > > --- > > arch/x86/kernel/smpboot.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c > > index 3f3ea0287f69..c3de98224cb4 100644 > > --- a/arch/x86/kernel/smpboot.c > > +++ b/arch/x86/kernel/smpboot.c > > @@ -545,7 +545,7 @@ static int x86_core_flags(void) > > #ifdef CONFIG_SCHED_SMT > > static int x86_smt_flags(void) > > { > > - return cpu_smt_flags() | x86_sched_itmt_flags(); > > + return cpu_smt_flags(); > > Based on: > > kernel/sched/topology.c: > sd = highest_flag_domain(cpu, SD_ASYM_PACKING); > rcu_assign_pointer(per_cpu(sd_asym_packing, cpu), sd); > > and described at: > > include/linux/sched/sd_flags.h: > /* > * 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()). > * NEEDS_GROUPS: Load balancing flag. > */ > SD_FLAG(SD_ASYM_PACKING, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS) > > doesn't your change result in sd_asym_packing being NULL?
Yes. This is a good catch. Thanks!
> > The SD_ASYM_PACKING flag requires all children of a domain to have it set > as well. So having SMT not setting the flag, while CLUSTER and MC having > set the flag would result in a broken topology, right?
I'd say that highest_flag_domain(..., flag) requires all children to have `flag`, but clearly the comment you quote allows for SD_ASYM_PACKING to be located in upper domains.
Perhaps this can be fixed with a variant of highest_flag_domain() that do not require all children to have the flag?
Thanks and BR, Ricardo
| |