| 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: <ZLAAEnd2HOinKrA+@righiandr-XPS-13-7390>
Date: Thu, 13 Jul 2023 15:45:54 +0200
From: Andrea Righi <andrea.righi@...onical.com>
To: Tejun Heo <tj@...nel.org>
Cc: torvalds@...ux-foundation.org, mingo@...hat.com,
peterz@...radead.org, juri.lelli@...hat.com,
vincent.guittot@...aro.org, dietmar.eggemann@....com,
rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
bristot@...hat.com, vschneid@...hat.com, ast@...nel.org,
daniel@...earbox.net, andrii@...nel.org, martin.lau@...nel.org,
joshdon@...gle.com, brho@...gle.com, pjt@...gle.com,
derkling@...gle.com, haoluo@...gle.com, dvernet@...a.com,
dschatzberg@...a.com, dskarlat@...cmu.edu, riel@...riel.com,
linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
kernel-team@...a.com
Subject: Re: [PATCH 27/34] sched_ext: Implement SCX_KICK_WAIT
On Mon, Jul 10, 2023 at 03:13:45PM -1000, Tejun Heo wrote:
...
> + for_each_cpu_andnot(cpu, this_rq->scx.cpus_to_wait,
> + cpumask_of(this_cpu)) {
> + /*
> + * Pairs with smp_store_release() issued by this CPU in
> + * scx_notify_pick_next_task() on the resched path.
> + *
> + * We busy-wait here to guarantee that no other task can be
> + * scheduled on our core before the target CPU has entered the
> + * resched path.
> + */
> + while (smp_load_acquire(&cpu_rq(cpu)->scx.pnt_seq) == pseqs[cpu])
> + cpu_relax();
> + }
> +
...
> +static inline void scx_notify_pick_next_task(struct rq *rq,
> + const struct task_struct *p,
> + const struct sched_class *active)
> +{
> +#ifdef CONFIG_SMP
> + if (!scx_enabled())
> + return;
> + /*
> + * Pairs with the smp_load_acquire() issued by a CPU in
> + * kick_cpus_irq_workfn() who is waiting for this CPU to perform a
> + * resched.
> + */
> + smp_store_release(&rq->scx.pnt_seq, rq->scx.pnt_seq + 1);
> +#endif
> +}
We can't use smp_load_acquire()/smp_store_release() with a u64 on
32-bit architectures.
For example, on armhf the build is broken:
In function ‘scx_notify_pick_next_task’,
inlined from ‘__pick_next_task’ at /<<PKGBUILDDIR>>/kernel/sched/core.c:6106:4,
inlined from ‘pick_next_task’ at /<<PKGBUILDDIR>>/kernel/sched/core.c:6605:9,
inlined from ‘__schedule’ at /<<PKGBUILDDIR>>/kernel/sched/core.c:6750:9:
/<<PKGBUILDDIR>>/include/linux/compiler_types.h:397:45: error: call to ‘__compiletime_assert_597’ declared with attribute error: Need native word sized stores/loads for atomicity.
397 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
/<<PKGBUILDDIR>>/include/linux/compiler_types.h:378:25: note: in definition of macro ‘__compiletime_assert’
378 | prefix ## suffix(); \
| ^~~~~~
/<<PKGBUILDDIR>>/include/linux/compiler_types.h:397:9: note: in expansion of macro ‘_compiletime_assert’
397 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
/<<PKGBUILDDIR>>/include/linux/compiler_types.h:400:9: note: in expansion of macro ‘compiletime_assert’
400 | compiletime_assert(__native_word(t), \
| ^~~~~~~~~~~~~~~~~~
/<<PKGBUILDDIR>>/include/asm-generic/barrier.h:141:9: note: in expansion of macro ‘compiletime_assert_atomic_type’
141 | compiletime_assert_atomic_type(*p); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/<<PKGBUILDDIR>>/include/asm-generic/barrier.h:172:55: note: in expansion of macro ‘__smp_store_release’
172 | #define smp_store_release(p, v) do { kcsan_release(); __smp_store_release(p, v); } while (0)
| ^~~~~~~~~~~~~~~~~~~
/<<PKGBUILDDIR>>/kernel/sched/ext.h:159:9: note: in expansion of macro ‘smp_store_release’
159 | smp_store_release(&rq->scx.pnt_seq, rq->scx.pnt_seq + 1);
There's probably a better way to fix this, but for now I've temporarily
solved this using cmpxchg64() - see patch below.
I'm not sure if we already have an equivalent of
smp_store_release_u64/smp_load_acquire_u64(). Otherwise, it may be worth
to add them to a more generic place.
-Andrea
diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 051c79fa25f7..5da72b1cf88d 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -3667,7 +3667,7 @@ static void kick_cpus_irq_workfn(struct irq_work *irq_work)
* scheduled on our core before the target CPU has entered the
* resched path.
*/
- while (smp_load_acquire(&cpu_rq(cpu)->scx.pnt_seq) == pseqs[cpu])
+ while (smp_load_acquire_u64(&cpu_rq(cpu)->scx.pnt_seq) == pseqs[cpu])
cpu_relax();
}
diff --git a/kernel/sched/ext.h b/kernel/sched/ext.h
index 405037a4e6ce..ef4a24d77d30 100644
--- a/kernel/sched/ext.h
+++ b/kernel/sched/ext.h
@@ -144,6 +144,40 @@ void __scx_notify_pick_next_task(struct rq *rq,
struct task_struct *p,
const struct sched_class *active);
+#ifdef CONFIG_64BIT
+static inline u64 smp_load_acquire_u64(u64 *ptr)
+{
+ return smp_load_acquire(ptr);
+}
+
+static inline void smp_store_release_u64(u64 *ptr, u64 val)
+{
+ smp_store_release(ptr, val);
+}
+#else
+static inline u64 smp_load_acquire_u64(u64 *ptr)
+{
+ u64 prev, next;
+
+ do {
+ prev = *ptr;
+ next = prev;
+ } while (cmpxchg64(ptr, prev, next) != prev);
+
+ return prev;
+}
+
+static inline void smp_store_release_u64(u64 *ptr, u64 val)
+{
+ u64 prev, next;
+
+ do {
+ prev = *ptr;
+ next = val;
+ } while (cmpxchg64(ptr, prev, next) != prev);
+}
+#endif
+
static inline void scx_notify_pick_next_task(struct rq *rq,
struct task_struct *p,
const struct sched_class *active)
@@ -156,7 +190,7 @@ static inline void scx_notify_pick_next_task(struct rq *rq,
* kick_cpus_irq_workfn() who is waiting for this CPU to perform a
* resched.
*/
- smp_store_release(&rq->scx.pnt_seq, rq->scx.pnt_seq + 1);
+ smp_store_release_u64(&rq->scx.pnt_seq, rq->scx.pnt_seq + 1);
#endif
if (!static_branch_unlikely(&scx_ops_cpu_preempt))
return;
--
2.40.1
Powered by blists - more mailing lists