linux-kernel - Re: [PATCH] sched/fair: favor non-idle group in tick preemption

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 Nov 2022 11:50:43 +0800
From:   Abel Wu <wuyun.abel@...edance.com>
To:     Josh Don <joshdon@...gle.com>, Chuyi Zhou <zhouchuyi@...edance.com>
Cc:     peterz@...radead.org, juri.lelli@...hat.com, mingo@...hat.com,
        vincent.guittot@...aro.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sched/fair: favor non-idle group in tick preemption

On 11/2/22 7:39 AM, Josh Don wrote:
>>> Some weirdness about this change though, is that if there is a
>>> non-idle current entity, and the two next entities on the cfs_rq are
>>> idle and non-idle respectively, we'll now take longer to preempt the
>>> on-cpu non-idle entity, because the non-idle entity on the cfs_rq is
>>> 'hidden' by the idle 'first' entity. Wakeup preemption is different
>>> because we're always directly comparing the current entity with the
>>> newly woken entity.
>>>
>> You are right, this can happen with high probability.
>> This patch just compared the curr with the first entity in
>> the tick, and it seems hard to consider all the other entity
>> in cfs_rq.
>>
>> So, what specific negative effects this situation would cause?
>> For example, the "hidden" non-idle entity's latency will be worse
>> than before?
> 
> As Abel points out in his email, it can push out the time it'll take
> to switch to the other non-idle entity. The change might boost some
> benchmarks numbers, but I don't think it is conclusive enough to say
> it is a generically beneficial improvement that should be integrated.

Agree.

> 
> By the way, I'm curious if you modified any of the sched_idle_cpu()
> and related load balancing around idle entities given that you've made
> it so that idle entities can have arbitrary weight (since, as I
> described in my prior email, this can otherwise cause issues there).

Being able to change idle entities' weight can bring nothing but
convenience, because it can also be achieved by modifying all their
siblings' weight. Which seems not a strong reason to get merged.

And I'm also thinking that, although rare, a non-idle group can also
have a weight close or even equal to 3. I guess some users who made
this kind of setting might only want to benefit from the preemption
at wakeup? Nevertheless this setting is supported now :)

Best,
	Abel