Re: [PATCH v6 3/4] sched/fair: Introduce SIS_CORE
From: Chen Yu
Date: Fri Oct 21 2022 - 00:03:43 EST
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?
[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'?
thanks,
Chenyu