Date:   Fri, 10 Feb 2023 17:53:03 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Valentin Schneider <vschneid@redhat.com>
Cc:     Ricardo Neri <ricardo.neri-calderon@linux.intel.com>,
        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>,
        Ionela Voinescu <ionela.voinescu@arm.com>, x86@kernel.org,
        linux-kernel@vger.kernel.org, "Tim C . Chen" <tim.c.chen@intel.com>
Subject: Re: [PATCH v3 06/10] sched/fair: Use the prefer_sibling flag of the
 current sched domain
Message-ID: <Y+Z2b/OtZDk9cT53@hirez.programming.kicks-ass.net>
References: <20230207045838.11243-1-ricardo.neri-calderon@linux.intel.com>
 <20230207045838.11243-7-ricardo.neri-calderon@linux.intel.com>
 <Y+YXrk5NRuWaSOGR@hirez.programming.kicks-ass.net>
 <xhsmhmt5lr2nz.mognet@vschneid.remote.csb>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <xhsmhmt5lr2nz.mognet@vschneid.remote.csb>
Precedence: bulk

On Fri, Feb 10, 2023 at 02:54:56PM +0000, Valentin Schneider wrote:

> So something like have SD_PREFER_SIBLING affect the SD it's on (and not
> its parent), but remove it from the lowest non-degenerated topology level?

So I was rather confused about the whole moving it between levels things
this morning -- conceptually, prefer siblings says you want to try
sibling domains before filling up your current domain. Now, balancing
between siblings happens one level up, hence looking at child->flags
makes perfect sense.

But looking at the current domain and still calling it prefer sibling
makes absolutely no sense what so ever.

In that confusion I think I also got the polarity wrong, I thought you
wanted to kill prefer_sibling for the assymetric SMT cases, instead you
want to force enable it as long as there is one SMT child around.

Whichever way around it we do it, I'm thinking perhaps some renaming
might be in order to clarify things.

How about adding a flag SD_SPREAD_TASKS, which is the effective toggle
of the behaviour, but have it be set by children with SD_PREFER_SIBLING
or something.

OTOH, there's also

  if (busiest->group_weight == 1 || sds->prefer_sibling) {

which explicitly also takes the group-of-one (the !child case) into
account, but that's not consistently done.

	sds->prefer_sibling = !child || child->flags & SD_PREFER_SIBLING;

seems an interesting option, except perhaps ASYM_CPUCAPACITY -- I
forget, can CPUs of different capacity be in the same leaf domain? With
big.LITTLE I think not, they had their own cache domains and so you get
at least MC domains per capacity, but DynamiQ might have totally wrecked
that party.

> (+ add it to the first NUMA level to keep things as they are, even if TBF I
> find relying on it for NUMA balancing a bit odd).

Arguably it ought to perhaps be one of those node_reclaim_distance
things. The thing is that NUMA-1 is often fairly quick, esp. these days
where it's basically on die numa.