Re: [PATCH v3 1/3] sched/fair: Add asymmetric CPU capacity wakeup scan

[Qais Yousef]

On 01/29/20 09:22, Pavan Kondeti wrote:
> On Tue, Jan 28, 2020 at 11:30:26AM +0000, Valentin Schneider wrote:
> > Hi Pavan,
> > 
> > On 28/01/2020 06:22, Pavan Kondeti wrote:
> > > Hi Valentin,
> > > 
> > > On Sun, Jan 26, 2020 at 08:09:32PM +0000, Valentin Schneider wrote:
> > >>  
> > >> +static inline int check_cpu_capacity(struct rq *rq, struct sched_domain *sd);
> > >> +
> > >> +/*
> > >> + * Scan the asym_capacity domain for idle CPUs; pick the first idle one on which
> > >> + * the task fits. If no CPU is big enough, but there are idle ones, try to
> > >> + * maximize capacity.
> > >> + */
> > >> +static int select_idle_capacity(struct task_struct *p, int target)
> > >> +{
> > >> +	unsigned long best_cap = 0;
> > >> +	struct sched_domain *sd;
> > >> +	struct cpumask *cpus;
> > >> +	int best_cpu = -1;
> > >> +	struct rq *rq;
> > >> +	int cpu;
> > >> +
> > >> +	if (!static_branch_unlikely(&sched_asym_cpucapacity))
> > >> +		return -1;
> > >> +
> > >> +	sd = rcu_dereference(per_cpu(sd_asym_cpucapacity, target));
> > >> +	if (!sd)
> > >> +		return -1;
> > >> +
> > >> +	sync_entity_load_avg(&p->se);
> > >> +
> > >> +	cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
> > >> +	cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
> > >> +
> > >> +	for_each_cpu_wrap(cpu, cpus, target) {
> > >> +		rq = cpu_rq(cpu);
> > >> +
> > >> +		if (!available_idle_cpu(cpu))
> > >> +			continue;
> > >> +		if (task_fits_capacity(p, rq->cpu_capacity))
> > >> +			return cpu;
> > > 
> > > I have couple of questions.
> > > 
> > > (1) Any particular reason for not checking sched_idle_cpu() as a backup
> > > for the case where all eligible CPUs are busy? select_idle_cpu() does
> > > that.
> > > 
> > 
> > No particular reason other than we didn't consider it, I think. I don't see
> > any harm in folding it in, I'll do that for v4. I am curious however; are
> > you folks making use of SCHED_IDLE? AFAIA Android isn't making use of it
> > yet, though Viresh paved the way for that to happen.
> > 
> 
> We are not using SCHED_IDLE in product setups. I am told Android may use it
> for background tasks in future. I am not completely sure though. I asked it
> because select_idle_cpu() is using it.

I believe Viresh intention when he pushed for the support was allowing this use
case.

FWIW, I had a patch locally to implement this but waiting on latency_nice
attribute to go in [1] before I attempt to upstream it.

The design goals I had in mind:

	1. If there are multiple energy efficient CPUs we can select, select
	   the idle one.
	2. If latency nice is set and the most energy efficient CPU is not
	   idle, then fallback to the most energy efficient idle CPU.

Android use case needs EAS path support before it can be useful to it.

[1] https://lore.kernel.org/lkml/20190919144259.vpuv7hvtqon4qgrv@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx/

--
Qais Yousef