| 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: <87sfvjavqk.mognet@arm.com>
Date: Fri, 26 Nov 2021 13:32:03 +0000
From: Valentin Schneider <Valentin.Schneider@....com>
To: Vincent Guittot <vincent.guittot@...aro.org>
Cc: Vincent Donnefort <Vincent.Donnefort@....com>,
peterz@...radead.org, mingo@...hat.com,
linux-kernel@...r.kernel.org, mgorman@...hsingularity.net,
dietmar.eggemann@....com
Subject: Re: [PATCH] sched/fair: Fix detection of per-CPU kthreads waking a task
On 26/11/21 09:23, Vincent Guittot wrote:
> On Thu, 25 Nov 2021 at 16:30, Valentin Schneider
> <Valentin.Schneider@....com> wrote:
>> On 25/11/21 14:23, Vincent Guittot wrote:
>> > If we want to filter wakeup
>> > generated by interrupt context while a per cpu kthread is running, it
>> > would be better to fix all cases and test the running context like
>> > this
>> >
>>
>> I think that could make sense - though can the idle task issue wakeups in
>> process context? If so that won't be sufficient. A quick audit tells me:
>>
>> o rcu_nocb_flush_deferred_wakeup() happens before calling into cpuidle
>> o I didn't see any wakeup issued from the cpu_pm_notifier call chain
>> o I'm not entirely sure about flush_smp_call_function_from_idle(). I found
>> this thing in RCU:
>>
>> smp_call_function_single(cpu, rcu_exp_handler)
>>
>> rcu_exp_handler()
>> rcu_report_exp_rdp()
>> rcu_report_exp_cpu_mult()
>> __rcu_report_exp_rnp()
>> swake_up_one()
>>
>> IIUC if set_nr_if_polling() then the smp_call won't send an IPI and should be
>> handled in that flush_foo_from_idle() call.
>
> Aren't all these planned to wakeup on local cpu ? so i don't see any
> real problem there
>
Hm so other than boot time oddities I think that does end up with threads
of an !UNBOUND (so pcpu) workqueue...
>>
>> I'd be tempted to stick your VincentD's conditions together, just to be
>> safe...
>
> More than safe I would prefer that we fix the correct root cause
> instead of hiding it
>
I did play around a bit to see if this could be true when evaluating that
is_per_cpu_kthread() condition:
is_idle_task(current) && in_task() && p->nr_cpus_allowed > 1
but no luck so far. An in_task() check would appear sufficient, but how's
this?
---
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 884f29d07963..f45806b7f47a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6390,14 +6390,18 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
return prev;
/*
- * Allow a per-cpu kthread to stack with the wakee if the
- * kworker thread and the tasks previous CPUs are the same.
- * The assumption is that the wakee queued work for the
- * per-cpu kthread that is now complete and the wakeup is
- * essentially a sync wakeup. An obvious example of this
+ * Allow a per-cpu kthread to stack with the wakee if the kworker thread
+ * and the tasks previous CPUs are the same. The assumption is that the
+ * wakee queued work for the per-cpu kthread that is now complete and
+ * the wakeup is essentially a sync wakeup. An obvious example of this
* pattern is IO completions.
+ *
+ * Ensure the wakeup is issued by the kthread itself, and don't match
+ * against the idle task because that could override the
+ * available_idle_cpu(target) check done higher up.
*/
- if (is_per_cpu_kthread(current) &&
+ if (is_per_cpu_kthread(current) && !is_idle_task(current) &&
+ in_task() &&
prev == smp_processor_id() &&
this_rq()->nr_running <= 1) {
return prev;
Powered by blists - more mailing lists