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;
Date:   Tue, 27 Sep 2022 13:01:32 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Cc:     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>,
        Valentin Schneider <vschneid@redhat.com>, x86@kernel.org,
        linux-kernel@vger.kernel.org, "Tim C . Chen" <tim.c.chen@intel.com>
Subject: Re: [RFC PATCH 09/23] sched/fair: Use task-class performance score
 to pick the busiest group
Message-ID: <YzLYDPU+upHeUG65@hirez.programming.kicks-ass.net>
References: <20220909231205.14009-1-ricardo.neri-calderon@linux.intel.com>
 <20220909231205.14009-10-ricardo.neri-calderon@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220909231205.14009-10-ricardo.neri-calderon@linux.intel.com>
Precedence: bulk

On Fri, Sep 09, 2022 at 04:11:51PM -0700, Ricardo Neri wrote:
> update_sd_pick_busiest() keeps on selecting as the busiest group scheduling
> groups of identical priority. Since both groups have the same priority,
> either group is a good choice. The classes of tasks in the scheduling
> groups can break this tie.
> 
> Pick as busiest the scheduling group that yields a higher task-class
> performance score after load balancing.

> +/**
> + * sched_asym_class_pick - Select a sched group based on classes of tasks
> + * @a:		A scheduling group
> + * @b:		A second scheduling group
> + * @a_stats:	Load balancing statistics of @a
> + * @b_stats:	Load balancing statistics of @b
> + *
> + * Returns: true if @a has the same priority and @a has classes of tasks that
> + * yield higher overall throughput after load balance. Returns false otherwise.
> + */
> +static bool sched_asym_class_pick(struct sched_group *a,
> +				  struct sched_group *b,
> +				  struct sg_lb_stats *a_stats,
> +				  struct sg_lb_stats *b_stats)
> +{
> +	/*
> +	 * Only use the class-specific preference selection if both sched
> +	 * groups have the same priority.
> +	 */
> +	if (arch_asym_cpu_priority(a->asym_prefer_cpu) !=
> +	    arch_asym_cpu_priority(b->asym_prefer_cpu))
> +		return false;
> +
> +	return sched_asym_class_prefer(a_stats, b_stats);
> +}
> +
>  #else /* CONFIG_SCHED_TASK_CLASSES */
>  static void update_rq_task_classes_stats(struct sg_lb_task_class_stats *class_sgs,
>  					 struct rq *rq)

> @@ -9049,6 +9111,12 @@ static bool update_sd_pick_busiest(struct lb_env *env,
>  		/* Prefer to move from lowest priority CPU's work */
>  		if (sched_asym_prefer(sg->asym_prefer_cpu, sds->busiest->asym_prefer_cpu))
>  			return false;
> +
> +		/* @sg and @sds::busiest have the same priority. */
> +		if (sched_asym_class_pick(sds->busiest, sg, &sds->busiest_stat, sgs))
> +			return false;
> +
> +		/* @sg has lower priority than @sds::busiest. */
>  		break;
>  
>  	case group_misfit_task:

So why does only this one instance of asym_prefer() require tie
breaking?

I must also re-iterate how much I hate having two different means of
dealing with big-little topologies.

And while looking through this, I must ask about the comment that goes
with sched_set_itmt_core_prio() vs the sg->asym_prefer_cpu assignment in
init_sched_groups_capacity(), what-up ?!