Received-SPF: pass (google.com: domain of linux-kernel+bounces-54721-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Hillf Danton <hdanton@sina.com>
To: Qais Yousef <qyousef@layalina.io>
Cc: Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	linux-kernel@vger.kernel.org,
	Pierre Gondois <Pierre.Gondois@arm.com>
Subject: Re: [PATCH v5 2/2] sched/fair: Check a task has a fitting cpu when updating misfit
Date: Tue,  6 Feb 2024 18:55:30 +0800
Message-Id: <20240206105530.763-1-hdanton@sina.com>
In-Reply-To: <20240205021123.2225933-3-qyousef@layalina.io>
References: <20240205021123.2225933-1-qyousef@layalina.io>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

On Mon,  5 Feb 2024 02:11:23 +0000 Qais Yousef <qyousef@layalina.io>
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5092,24 +5092,36 @@ static inline int task_fits_cpu(struct task_struct *p, int cpu)
>  
>  static inline void update_misfit_status(struct task_struct *p, struct rq *rq)
>  {
> +	unsigned long cpu_cap;
> +	int cpu = cpu_of(rq);
> +
>  	if (!sched_asym_cpucap_active())
>  		return;
>  
> -	if (!p || p->nr_cpus_allowed == 1) {
> -		rq->misfit_task_load = 0;
> -		return;
> -	}
> +	if (!p || p->nr_cpus_allowed == 1)
> +		goto out;
>  
> -	if (task_fits_cpu(p, cpu_of(rq))) {
> -		rq->misfit_task_load = 0;
> -		return;
> -	}
> +	cpu_cap = arch_scale_cpu_capacity(cpu);
> +
> +	/* If we can't fit the biggest CPU, that's the best we can ever get. */
> +	if (cpu_cap == rq->rd->max_cpu_capacity)
> +		goto out;
> +
> +	/* Affinity allows us to go somewhere higher? */
> +	if (cpu_cap == p->max_allowed_capacity)
> +		goto out;

Looks good.
> +
> +	if (task_fits_cpu(p, cpu))
> +		goto out;
>  
>  	/*
>  	 * Make sure that misfit_task_load will not be null even if
>  	 * task_h_load() returns 0.
>  	 */
>  	rq->misfit_task_load = max_t(unsigned long, task_h_load(p), 1);
> +	return;
> +out:
> +	rq->misfit_task_load = 0;
>  }
>  
>  #else /* CONFIG_SMP */
> @@ -8241,6 +8253,36 @@ static void task_dead_fair(struct task_struct *p)
>  	remove_entity_load_avg(&p->se);
>  }
>  
> +/*
> + * Check the max capacity the task is allowed to run at for misfit detection.
> + */
> +static void set_task_max_allowed_capacity(struct task_struct *p)
> +{
> +	struct asym_cap_data *entry;
> +
> +	if (!sched_asym_cpucap_active())
> +		return;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(entry, &asym_cap_list, link) {
> +		cpumask_t *cpumask;
> +
> +		cpumask = cpu_capacity_span(entry);
> +		if (!cpumask_intersects(p->cpus_ptr, cpumask))
> +			continue;
> +
> +		p->max_allowed_capacity = entry->capacity;
> +		break;

Given what max_allowed_capacity could mean, it is needed to find the max
capacity by iterating the asym_cap_list instead of the first capacity
determined by the allowed CPU mask.
> +	}
> +	rcu_read_unlock();
> +}

BTW is it working in case of systems with a super core?

	cpu0-3	cpu4-6	cpu7
	little	big	super
	core	core	core