Re: [PATCH v6 3/4] sched/fair: Introduce SIS_CORE

[Abel Wu <wuyun.abel@bytedance.com>]

Hi Chen, thanks for your reviewing!

On 10/21/22 12:03 PM, Chen Yu wrote:
> On 2022-10-19 at 20:28:58 +0800, Abel Wu wrote:
> [cut]
>> A major concern is the accuracy of the idle cpumask. A cpu present
>> in the mask might not be idle any more, which is called the false
>> positive cpu. Such cpus will negate lots of benefit this feature
>> brings. The strategy against the false positives will be introduced
>> in next patch.
>>
> I was thinking that, if patch[3/4] needs [4/4] to fix the false positives,
> maybe SIS_CORE could be disabled by default in 3/4 but enabled
> in 4/4? So this might facilicate git bisect in case of any regression
> check?

Agreed. Will fix in next version.

> [cut]
>> + * To honor the rule of CORE granule update, set this cpu to the LLC idle
>> + * cpumask only if there is no cpu of this core showed up in the cpumask.
>> + */
>> +static void update_idle_cpu(int cpu)
>> +{
>> +	struct sched_domain_shared *sds;
>> +
>> +	if (!sched_feat(SIS_CORE))
>> +		return;
>> +
>> +	sds = rcu_dereference(per_cpu(sd_llc_shared, cpu));
>> +	if (sds) {
>> +		struct cpumask *icpus = to_cpumask(sds->icpus);
>> +
>> +		/*
>> +		 * This is racy against clearing in select_idle_cpu(),
>> +		 * and can lead to idle cpus miss the chance to be set to
>> +		 * the idle cpumask, thus the idle cpus are temporarily
>> +		 * out of reach in SIS domain scan. But it should be rare
>> +		 * and we still have ILB to kick them working.
>> +		 */
>> +		if (!cpumask_intersects(cpu_smt_mask(cpu), icpus))
>> +			cpumask_set_cpu(cpu, icpus);
> Maybe I miss something, here we only set one CPU in the icpus, but
> when we reach update_idle_cpu(), all SMT siblings of 'cpu' are idle,
> is this intended for 'CORE granule update'?

The __update_idle_core() is called by all the cpus that need to go idle
to update has_idle_core if necessary, and update_idle_cpu() is called
before that check.

Thanks,
Abel