| 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
| ||
|
Date: Thu, 29 Apr 2021 02:38:08 +0000
From: "Song Bao Hua (Barry Song)" <song.bao.hua@...ilicon.com>
To: Dietmar Eggemann <dietmar.eggemann@....com>,
Vincent Guittot <vincent.guittot@...aro.org>
CC: "tim.c.chen@...ux.intel.com" <tim.c.chen@...ux.intel.com>,
"catalin.marinas@....com" <catalin.marinas@....com>,
"will@...nel.org" <will@...nel.org>,
"rjw@...ysocki.net" <rjw@...ysocki.net>,
"bp@...en8.de" <bp@...en8.de>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
"lenb@...nel.org" <lenb@...nel.org>,
"rostedt@...dmis.org" <rostedt@...dmis.org>,
"bsegall@...gle.com" <bsegall@...gle.com>,
"mgorman@...e.de" <mgorman@...e.de>,
"msys.mizuma@...il.com" <msys.mizuma@...il.com>,
"valentin.schneider@....com" <valentin.schneider@....com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
Jonathan Cameron <jonathan.cameron@...wei.com>,
"juri.lelli@...hat.com" <juri.lelli@...hat.com>,
"mark.rutland@....com" <mark.rutland@....com>,
"sudeep.holla@....com" <sudeep.holla@....com>,
"aubrey.li@...ux.intel.com" <aubrey.li@...ux.intel.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"x86@...nel.org" <x86@...nel.org>, "xuwei (O)" <xuwei5@...wei.com>,
"Zengtao (B)" <prime.zeng@...ilicon.com>,
"guodong.xu@...aro.org" <guodong.xu@...aro.org>,
yangyicong <yangyicong@...wei.com>,
"Liguozhu (Kenneth)" <liguozhu@...ilicon.com>,
"hpa@...or.com" <hpa@...or.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"peterz@...radead.org" <peterz@...radead.org>
Subject: RE: Re: [RFC PATCH v6 3/4] scheduler: scan idle cpu in cluster for
tasks within one LLC
> >>
> >>> -----Original Message-----
> >>> From: Dietmar Eggemann [mailto:dietmar.eggemann@....com]
>
> [...]
>
> >>> On 20/04/2021 02:18, Barry Song wrote:
>
> [...]
>
> >> I am really confused. The whole code has only checked if wake_flags
> >> has WF_TTWU, it has never checked if sd_domain has SD_BALANCE_WAKE flag.
> >
> > look at :
> > #define WF_TTWU 0x08 /* Wakeup; maps to SD_BALANCE_WAKE */
> >
> > so when wake_wide return false, we use the wake_affine mecanism but
> > if it's false then we fllback to default mode which looks for:
> > if (tmp->flags & sd_flag)
> >
> > This means looking for SD_BALANCE_WAKE which is never set
> >
> > so sd will stay NULL and you will end up calling select_idle_sibling anyway
> >
> >>
> >> static int
> >> select_task_rq_fair(struct task_struct *p, int prev_cpu, int wake_flags)
> >> {
> >> ...
> >>
> >> if (wake_flags & WF_TTWU) {
> >> record_wakee(p);
> >>
> >> if (sched_energy_enabled()) {
> >> new_cpu = find_energy_efficient_cpu(p, prev_cpu);
> >> if (new_cpu >= 0)
> >> return new_cpu;
> >> new_cpu = prev_cpu;
> >> }
> >>
> >> want_affine = !wake_wide(p) && cpumask_test_cpu(cpu,
> p->cpus_ptr);
> >> }
> >> }
> >>
> >> And try_to_wake_up() has always set WF_TTWU:
> >> static int
> >> try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
> >> {
> >> cpu = select_task_rq(p, p->wake_cpu, wake_flags | WF_TTWU);
> >> ...
> >> }
> >>
> >> So the change in wake_wide will actually affect the value of want_affine.
> >> And I did also see code entered slow path during my benchmark.
>
> Yes, this is happening but IMHO not for wakeups. Check wake_flags for
> the tasks which go through `slow path` on your machine. They should have
> WF_EXEC or WF_FORK, not WF_TTWU (& WF_SYNC).
Yes. Both of you are right. The slow path I reported yesterday came from
WF_FORK actually.
>
> >> One issue I mentioned during linaro open discussion is that
> >> since I have moved to use cluster size to decide the value
> >> of wake_wide, relatively less tasks will make wake_wide()
> >> decide to go to slow path, thus, tasks begin to spread to
> >> other NUMA, but actually llc_size might be able to contain
> >> those tasks. So a possible model might be:
> >> static int wake_wide(struct task_struct *p)
> >> {
> >> tasksize < cluster : scan cluster
> >> tasksize > llc : slow path
> >> tasksize > cluster && tasksize < llc: scan llc
> >> }
> >>
> >> thoughts?
>
> Like Vincent explained, the return value of wake_wide() doesn't matter.
> For wakeups you always end up in sis().
Though we will never go to slow path, wake_wide() will affect want_affine,
so eventually affect the "new_cpu"?
for_each_domain(cpu, tmp) {
/*
* If both 'cpu' and 'prev_cpu' are part of this domain,
* cpu is a valid SD_WAKE_AFFINE target.
*/
if (want_affine && (tmp->flags & SD_WAKE_AFFINE) &&
cpumask_test_cpu(prev_cpu, sched_domain_span(tmp))) {
if (cpu != prev_cpu)
new_cpu = wake_affine(tmp, p, cpu, prev_cpu, sync);
sd = NULL; /* Prefer wake_affine over balance flags */
break;
}
if (tmp->flags & sd_flag)
sd = tmp;
else if (!want_affine)
break;
}
If wake_affine is false, the above won't execute, new_cpu(target) will
always be "prev_cpu"? so when "task size > cluster size" in wake_wide(),
this means we won't pull the wakee to the cluster of waker since target
is always prev_cpu? It seems sensible.
Thanks
Barry
Powered by blists - more mailing lists