| 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
| ||
|
Message-ID: <a7dca9ec-9f7b-4d7f-aa3f-9a68cf6430a7@linux.ibm.com>
Date: Fri, 18 Apr 2025 23:25:19 +0530
From: Shrikanth Hegde <sshegde@...ux.ibm.com>
To: Vincent Guittot <vincent.guittot@...aro.org>,
K Prateek Nayak <kprateek.nayak@....com>
Cc: "Chen, Yu C" <yu.c.chen@...el.com>, Tim Chen
<tim.c.chen@...ux.intel.com>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>,
Doug Nelson <doug.nelson@...el.com>,
Mohini Narkhede <mohini.narkhede@...el.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sched: Skip useless sched_balance_running acquisition if
load balance is not due
On 4/18/25 20:32, Vincent Guittot wrote:
> On Wed, 16 Apr 2025 at 16:14, Shrikanth Hegde <sshegde@...ux.ibm.com> wrote:
>>
>>
>>
>> On 4/16/25 15:17, Vincent Guittot wrote:
>>> On Wed, 16 Apr 2025 at 11:29, Shrikanth Hegde <sshegde@...ux.ibm.com> wrote:
>>>>
>>>>
>>>>
>>>> On 4/16/25 14:46, Shrikanth Hegde wrote:
>>>>>
>>>>>
>>>>> On 4/16/25 11:58, Chen, Yu C wrote:
>>>>>> Hi Shrikanth,
>>>>>>
>>>>>> On 4/16/2025 1:30 PM, Shrikanth Hegde wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 4/16/25 09:28, Tim Chen wrote:
>>>>>>>> At load balance time, balance of last level cache domains and
>>>>>>>> above needs to be serialized. The scheduler checks the atomic var
>>>>>>>> sched_balance_running first and then see if time is due for a load
>>>>>>>> balance. This is an expensive operation as multiple CPUs can attempt
>>>>>>>> sched_balance_running acquisition at the same time.
>>>>>>>>
>>>>>>>> On a 2 socket Granite Rapid systems enabling sub-numa cluster and
>>>>>>>> running OLTP workloads, 7.6% of cpu cycles are spent on cmpxchg of
>>>>>>>> sched_balance_running. Most of the time, a balance attempt is aborted
>>>>>>>> immediately after acquiring sched_balance_running as load balance time
>>>>>>>> is not due.
>>>>>>>>
>>>>>>>> Instead, check balance due time first before acquiring
>>>>>>>> sched_balance_running. This skips many useless acquisitions
>>>>>>>> of sched_balance_running and knocks the 7.6% CPU overhead on
>>>>>>>> sched_balance_domain() down to 0.05%. Throughput of the OLTP workload
>>>>>>>> improved by 11%.
>>>>>>>>
>>>>>>>
>>>>>>> Hi Tim.
>>>>>>>
>>>>>>> Time check makes sense specially on large systems mainly due to
>>>>>>> NEWIDLE balance.
>>>>>
>>>>> scratch the NEWLY_IDLE part from that comment.
>>>>>
>>>>>>>
>>>>>>
>>>>>> Could you elaborate a little on this statement? There is no timeout
>>>>>> mechanism like periodic load balancer for the NEWLY_IDLE, right?
>>>>>
>>>>> Yes. NEWLY_IDLE is very opportunistic.
>>>>>
>>>>>>
>>>>>>> One more point to add, A lot of time, the CPU which acquired
>>>>>>> sched_balance_running,
>>>>>>> need not end up doing the load balance, since it not the CPU meant to
>>>>>>> do the load balance.
>>>>>>>
>>>>>>> This thread.
>>>>>>> https://lore.kernel.org/all/1e43e783-55e7-417f-
>>>>>>> a1a7-503229eb163a@...ux.ibm.com/
>>>>>>>
>>>>>>>
>>>>>>> Best thing probably is to acquire it if this CPU has passed the time
>>>>>>> check and as well it is
>>>>>>> actually going to do load balance.
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> This is a good point, and we might only want to deal with periodic load
>>>>>> balancer rather than NEWLY_IDLE balance. Because the latter is too
>>>>>> frequent and contention on the sched_balance_running might introduce
>>>>>> high cache contention.
>>>>>>
>>>>>
>>>>> But NEWLY_IDLE doesn't serialize using sched_balance_running and can
>>>>> endup consuming a lot of cycles. But if we serialize using
>>>>> sched_balance_running, it would definitely cause a lot contention as is.
>>>>>
>>>>>
>>>>> The point was, before acquiring it, it would be better if this CPU is
>>>>> definite to do the load balance. Else there are chances to miss the
>>>>> actual load balance.
>>>>>
>>>>>
>>>>
>>>> Sorry, forgot to add.
>>>>
>>>> Do we really need newidle running all the way till NUMA? or if it runs till PKG is it enough?
>>>> the regular (idle) can take care for NUMA by serializing it?
>>>>
>>>> - if (sd->flags & SD_BALANCE_NEWIDLE) {
>>>> + if (sd->flags & SD_BALANCE_NEWIDLE && !(sd->flags & SD_SERIALIZE)) {
>>>
>>> Why not just clearing SD_BALANCE_NEWIDLE in your sched domain when you
>>> set SD_SERIALIZE
>>
>> Hi Vincent.
>>
>> There is even kernel parameter "relax_domain_level" which one can make use of.
>> concern was newidle does this without acquiring the sched_balance_running,
>> while busy,idle try to acquire this for NUMA.
>>
>>
>>
>> Slightly different topic: It(kernel parameter) also resets SHCED_BALANCE_WAKE. But is it being used?
>> I couldn't find out how it is used.
>
> Hi Shrikanth,
>
> The define below does the link
>
> #define WF_TTWU 0x08 /* Wakeup; maps to SD_BALANCE_WAKE */
>
> int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
> ..
> wake_flags |= WF_TTWU;
> ..
> cpu = select_task_rq(p, p->wake_cpu, &wake_flags);
> select_task_rq_fair()
> int sd_flag = wake_flags & 0xF;
> ..
> for_each_domain(cpu, tmp) {
> ..
> if (tmp->flags & sd_flag)
>
Thanks Vincent, Prateek.
>>
>>>
>>>>
>>>> pulled_task = sched_balance_rq(this_cpu, this_rq,
>>>> sd, CPU_NEWLY_IDLE,
>>>>
>>>>
>>>> Anyways, having a policy around this SD_SERIALIZE would be a good thing.
>>>>
>>>>>> thanks,
>>>>>> Chenyu
>>>>>>
>>>>>>>> Signed-off-by: Tim Chen <tim.c.chen@...ux.intel.com>
>>>>>>>> Reported-by: Mohini Narkhede <mohini.narkhede@...el.com>
>>>>>>>> Tested-by: Mohini Narkhede <mohini.narkhede@...el.com>
>>>>>>>> ---
>>>>>>>> kernel/sched/fair.c | 16 ++++++++--------
>>>>>>>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>>>>>>> index e43993a4e580..5e5f7a770b2f 100644
>>>>>>>> --- a/kernel/sched/fair.c
>>>>>>>> +++ b/kernel/sched/fair.c
>>>>>>>> @@ -12220,13 +12220,13 @@ static void sched_balance_domains(struct
>>>>>>>> rq *rq, enum cpu_idle_type idle)
>>>>>>>> interval = get_sd_balance_interval(sd, busy);
>>>>>>>> - need_serialize = sd->flags & SD_SERIALIZE;
>>>>>>>> - if (need_serialize) {
>>>>>>>> - if (atomic_cmpxchg_acquire(&sched_balance_running, 0, 1))
>>>>>>>> - goto out;
>>>>>>>> - }
>>>>>>>> -
>>>>>>>> if (time_after_eq(jiffies, sd->last_balance + interval)) {
>>>>>>>> + need_serialize = sd->flags & SD_SERIALIZE;
>>>>>>>> + if (need_serialize) {
>>>>>>>> + if (atomic_cmpxchg_acquire(&sched_balance_running,
>>>>>>>> 0, 1))
>>>>>>>> + goto out;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> if (sched_balance_rq(cpu, rq, sd, idle,
>>>>>>>> &continue_balancing)) {
>>>>>>>> /*
>>>>>>>> * The LBF_DST_PINNED logic could have changed
>>>>>>>> @@ -12238,9 +12238,9 @@ static void sched_balance_domains(struct rq
>>>>>>>> *rq, enum cpu_idle_type idle)
>>>>>>>> }
>>>>>>>> sd->last_balance = jiffies;
>>>>>>>> interval = get_sd_balance_interval(sd, busy);
>>>>>>>> + if (need_serialize)
>>>>>>>> + atomic_set_release(&sched_balance_running, 0);
>>>>>>>> }
>>>>>>>> - if (need_serialize)
>>>>>>>> - atomic_set_release(&sched_balance_running, 0);
>>>>>>>> out:
>>>>>>>> if (time_after(next_balance, sd->last_balance + interval)) {
>>>>>>>> next_balance = sd->last_balance + interval;
>>>>>>>
>>>>>
>>>>
>>
Powered by blists - more mailing lists