lkml.org 
[lkml]   [2022]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v4 6/7] sched/fair: skip busy cores in SIS search
From
On 7/11/22 8:02 PM, Chen Yu Wrote:
>>> ...
>>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>>> index d3e2c5a7c1b7..452eb63ee6f6 100644
>>> --- a/kernel/sched/core.c
>>> +++ b/kernel/sched/core.c
>>> @@ -5395,6 +5395,7 @@ void scheduler_tick(void)
>>> resched_latency = cpu_resched_latency(rq);
>>> calc_global_load_tick(rq);
>>> sched_core_tick(rq);
>>> + update_overloaded_rq(rq);
>>
>> I didn't see this update in idle path. Is this on your intend?
>>
> It is intended to exclude the idle path. My thought was that, since
> the avg_util has contained the historic activity, checking the cpu
> status in each idle path seems to have no much difference...

I presume the avg_util is used to decide how many cpus to scan, while
the update determines which cpus to scan.

>>> rq_unlock(rq, &rf);
>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>> index f80ae86bb404..34b1650f85f6 100644
>>> --- a/kernel/sched/fair.c
>>> +++ b/kernel/sched/fair.c
>>> @@ -6323,6 +6323,50 @@ static inline int select_idle_smt(struct task_struct *p, struct sched_domain *sd
>>> #endif /* CONFIG_SCHED_SMT */
>>> +/* derived from group_is_overloaded() */
>>> +static inline bool rq_overloaded(struct rq *rq, int cpu, unsigned int imbalance_pct)
>>> +{
>>> + if (rq->nr_running - rq->cfs.idle_h_nr_running <= 1)
>>> + return false;
>>> +
>>> + if ((SCHED_CAPACITY_SCALE * 100) <
>>> + (cpu_util_cfs(cpu) * imbalance_pct))
>>> + return true;
>>> +
>>> + if ((SCHED_CAPACITY_SCALE * imbalance_pct) <
>>> + (cpu_runnable(rq) * 100))
>>> + return true;
>>
>> So the filter contains cpus that over-utilized or overloaded now.
>> This steps further to make the filter reliable while at the cost
>> of sacrificing scan efficiency.
>>
> Right. Ideally if there is a 'realtime' idle cpumask for SIS, the
> scan would be quite accurate. The issue is how to maintain this
> cpumask in low cost.

Yes indeed.

>> The idea behind my recent patches is to keep the filter radical,
>> but use it conservatively.
>>
> Do you mean, update the per-core idle filter frequently, but only
> propogate the filter to LLC-cpumask when the system is overloaded?

Not exactly. I want to update the filter (BTW there is only the LLC
filter, no core filters :)) once core state changes, while apply it
in SIS domain scan only if the domain is busy enough.

>>> +
>>> + return false;
>>> +}
>>> +
>>> +void update_overloaded_rq(struct rq *rq)
>>> +{
>>> + struct sched_domain_shared *sds;
>>> + struct sched_domain *sd;
>>> + int cpu;
>>> +
>>> + if (!sched_feat(SIS_FILTER))
>>> + return;
>>> +
>>> + cpu = cpu_of(rq);
>>> + sd = rcu_dereference(per_cpu(sd_llc, cpu));
>>> + if (unlikely(!sd))
>>> + return;
>>> +
>>> + sds = rcu_dereference(per_cpu(sd_llc_shared, cpu));
>>> + if (unlikely(!sds))
>>> + return;
>>> +
>>> + if (rq_overloaded(rq, cpu, sd->imbalance_pct)) {
>>
>> I'm not sure whether it is appropriate to use LLC imbalance pct here,
>> because we are comparing inside the LLC rather than between the LLCs.
>>
> Right, imbalance_pct could not be of LLC's, it could be of the core domain's
> imbalance_pct.
>>> + /* avoid duplicated write, mitigate cache contention */
>>> + if (!cpumask_test_cpu(cpu, sdo_mask(sds)))
>>> + cpumask_set_cpu(cpu, sdo_mask(sds));
>>> + } else {
>>> + if (cpumask_test_cpu(cpu, sdo_mask(sds)))
>>> + cpumask_clear_cpu(cpu, sdo_mask(sds));
>>> + }
>>> +}
>>> /*
>>> * Scan the LLC domain for idle CPUs; this is dynamically regulated by
>>> * comparing the average scan cost (tracked in sd->avg_scan_cost) against the
>>> @@ -6383,6 +6427,9 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool
>>> }
>>> }
>>> + if (sched_feat(SIS_FILTER) && !has_idle_core && sd->shared)
>>> + cpumask_andnot(cpus, cpus, sdo_mask(sd->shared));
>>> +
>>> for_each_cpu_wrap(cpu, cpus, target + 1) {
>>> if (has_idle_core) {
>>> i = select_idle_core(p, cpu, cpus, &idle_cpu);
>>> diff --git a/kernel/sched/features.h b/kernel/sched/features.h
>>> index ee7f23c76bd3..1bebdb87c2f4 100644
>>> --- a/kernel/sched/features.h
>>> +++ b/kernel/sched/features.h
>>> @@ -62,6 +62,7 @@ SCHED_FEAT(TTWU_QUEUE, true)
>>> */
>>> SCHED_FEAT(SIS_PROP, false)
>>> SCHED_FEAT(SIS_UTIL, true)
>>> +SCHED_FEAT(SIS_FILTER, true)
>>
>> The filter should be enabled when there is a need. If the system
>> is idle enough, I don't think it's a good idea to clear out the
>> overloaded cpus from domain scan. Making the filter a sched-feat
>> won't help the problem.
>>
>> My latest patch will only apply the filter when nr is less than
>> the LLC size.
> Do you mean only update the filter(idle cpu mask), or only uses the
> filter in SIS when the system meets: nr_running < LLC size?
>

In SIS domain search, apply the filter when nr < LLC_size. But I
haven't tested this with SIS_UTIL, and in the SIS_UTIL case this
condition seems always true.

Thanks,
Abel

\
 
 \ /
  Last update: 2022-07-13 12:26    [W:0.081 / U:0.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site