Re: [RFC PATCH] sched/fair: Choose the CPU where short task is running during wake up

From: Tim Chen <tim.c.chen@linux.intel.com>
To: Chen Yu <yu.c.chen@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Tim Chen <tim.c.chen@intel.com>,
	Mel Gorman <mgorman@techsingularity.net>
Cc: Juri Lelli <juri.lelli@redhat.com>,
	Rik van Riel <riel@surriel.com>, Aaron Lu <aaron.lu@intel.com>,
	Abel Wu <wuyun.abel@bytedance.com>,
	K Prateek Nayak <kprateek.nayak@amd.com>,
	Yicong Yang <yangyicong@hisilicon.com>,
	"Gautham R . Shenoy" <gautham.shenoy@amd.com>,
	Ingo Molnar <mingo@redhat.com>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Valentin Schneider <vschneid@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] sched/fair: Choose the CPU where short task is running during wake up
Date: Thu, 15 Sep 2022 10:10:25 -0700	[thread overview]
Message-ID: <bb7dd6ac32403e96943a6e51c60e72960c2942fd.camel@linux.intel.com> (raw)
In-Reply-To: <20220915165407.1776363-1-yu.c.chen@intel.com>

On Fri, 2022-09-16 at 00:54 +0800, Chen Yu wrote:
> 
> +/*
> + * If a task switches in and then voluntarily relinquishes the
> + * CPU quickly, it is regarded as a short running task.
> + * sysctl_sched_min_granularity is chosen as the threshold,
> + * as this value is the minimal slice if there are too many
> + * runnable tasks, see __sched_period().
> + */
> +static int is_short_task(struct task_struct *p)
> +{
> +	return (p->se.sum_exec_runtime <=
> +		(p->nvcsw * sysctl_sched_min_granularity));
> +}
> +
>  /*
>   * The purpose of wake_affine() is to quickly determine on which CPU we can run
>   * soonest. For the purpose of speed we only consider the waking and previous
> @@ -6050,7 +6063,8 @@ wake_affine_idle(int this_cpu, int prev_cpu, int sync)
>  	if (available_idle_cpu(this_cpu) && cpus_share_cache(this_cpu, prev_cpu))
>  		return available_idle_cpu(prev_cpu) ? prev_cpu : this_cpu;
>  
> -	if (sync && cpu_rq(this_cpu)->nr_running == 1)
> +	if ((sync && cpu_rq(this_cpu)->nr_running == 1) ||
> +	    is_short_task(cpu_curr(this_cpu)))
>  		return this_cpu;
>  
>  	if (available_idle_cpu(prev_cpu))
> @@ -6434,6 +6448,21 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool
>  			/* overloaded LLC is unlikely to have idle cpu/core */
>  			if (nr == 1)
>  				return -1;
> +
> +			/*
> +			 * If nr is smaller than 60% of llc_weight, it
> +			 * indicates that the util_avg% is higher than 50%.
> +			 * This is calculated by SIS_UTIL in
> +			 * update_idle_cpu_scan(). The 50% util_avg indicates
> +			 * a half-busy LLC domain. System busier than this
> +			 * level could lower its bar to choose a compromised
> +			 * "idle" CPU. If the waker on target CPU is a short
> +			 * task and the wakee is also a short task, pick
> +			 * target directly.
> +			 */
> +			if (!has_idle_core && (5 * nr < 3 * sd->span_weight) &&
> +			    is_short_task(p) && is_short_task(cpu_curr(target)))

Should we check if target's rq's nr_running is 1, and if there's pending waking
task before picking it?

> +				return target;
>  		}
>  	}
>  

Thanks.

Tim