* [PATCH 0/2] sched: Address idle task vs pcpu kthread checks
@ 2021-05-10 15:10 Valentin Schneider
2021-05-10 15:10 ` [PATCH 1/2] sched: Make the idle task quack like a per-CPU kthread Valentin Schneider
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Valentin Schneider @ 2021-05-10 15:10 UTC (permalink / raw)
To: linux-kernel; +Cc: mingo, peterz, tglx, bristot, yejune.deng
Commit
5ba2ffba13a1 ("sched: Fix CPU hotplug / tighten is_per_cpu_kthread()")
had to special-case the idle task when checking for per-CPU kthreads. This is
due to the idle task not having its own struct kthread, which is where we'd
store KTHREAD_IS_PER_CPU.
From staring at Yejune's recent patch [1], it turns out the idle task is also
missing PF_NO_SETAFFINITY.
Patch 1 cleans this up, patch 2 is Yejune's v1 which depends on it.
Note: I remember seeing some patch(es) from Peter tackling this exact
problem, but I couldn't find them again.
[1]: http://lore.kernel.org/r/1620458722-13026-1-git-send-email-yejunedeng@gmail.com
Cheers,
Valentin
Valentin Schneider (1):
sched: Make the idle task quack like a per-CPU kthread
Yejune Deng (1):
lib/smp_processor_id: Use is_percpu_thread() instead of
nr_cpus_allowed
include/linux/kthread.h | 2 ++
kernel/kthread.c | 30 ++++++++++++++++++------------
kernel/sched/core.c | 21 +++++++++++++++------
lib/smp_processor_id.c | 6 +-----
4 files changed, 36 insertions(+), 23 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/2] sched: Make the idle task quack like a per-CPU kthread 2021-05-10 15:10 [PATCH 0/2] sched: Address idle task vs pcpu kthread checks Valentin Schneider @ 2021-05-10 15:10 ` Valentin Schneider 2021-05-10 15:57 ` Valentin Schneider 2021-05-19 8:09 ` [tip: sched/core] " tip-bot2 for Valentin Schneider 2021-05-10 15:10 ` [PATCH 2/2] lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed Valentin Schneider 2021-05-12 11:00 ` [PATCH 0/2] sched: Address idle task vs pcpu kthread checks Peter Zijlstra 2 siblings, 2 replies; 13+ messages in thread From: Valentin Schneider @ 2021-05-10 15:10 UTC (permalink / raw) To: linux-kernel; +Cc: mingo, peterz, tglx, bristot, yejune.deng For all intents and purposes, the idle task is a per-CPU kthread. It isn't created via the same route as other pcpu kthreads however, and as a result it is missing a few bells and whistles: it fails kthread_is_per_cpu() and it doesn't have PF_NO_SETAFFINITY set. Fix the former by giving the idle task a kthread struct along with the KTHREAD_IS_PER_CPU flag. This requires some extra iffery as init_idle() call be called more than once on the same idle task. Signed-off-by: Valentin Schneider <valentin.schneider@arm.com> --- include/linux/kthread.h | 2 ++ kernel/kthread.c | 30 ++++++++++++++++++------------ kernel/sched/core.c | 21 +++++++++++++++------ 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/include/linux/kthread.h b/include/linux/kthread.h index 2484ed97e72f..d9133d6db308 100644 --- a/include/linux/kthread.h +++ b/include/linux/kthread.h @@ -33,6 +33,8 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data), unsigned int cpu, const char *namefmt); +void set_kthread_struct(struct task_struct *p); + void kthread_set_per_cpu(struct task_struct *k, int cpu); bool kthread_is_per_cpu(struct task_struct *k); diff --git a/kernel/kthread.c b/kernel/kthread.c index 6d3c488a0f82..77dd0ea1d35d 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -68,16 +68,6 @@ enum KTHREAD_BITS { KTHREAD_SHOULD_PARK, }; -static inline void set_kthread_struct(void *kthread) -{ - /* - * We abuse ->set_child_tid to avoid the new member and because it - * can't be wrongly copied by copy_process(). We also rely on fact - * that the caller can't exec, so PF_KTHREAD can't be cleared. - */ - current->set_child_tid = (__force void __user *)kthread; -} - static inline struct kthread *to_kthread(struct task_struct *k) { WARN_ON(!(k->flags & PF_KTHREAD)); @@ -103,6 +93,22 @@ static inline struct kthread *__to_kthread(struct task_struct *p) return kthread; } +void set_kthread_struct(struct task_struct *p) +{ + struct kthread *kthread; + + if (__to_kthread(p)) + return; + + kthread = kzalloc(sizeof(*kthread), GFP_KERNEL); + /* + * We abuse ->set_child_tid to avoid the new member and because it + * can't be wrongly copied by copy_process(). We also rely on fact + * that the caller can't exec, so PF_KTHREAD can't be cleared. + */ + p->set_child_tid = (__force void __user *)kthread; +} + void free_kthread_struct(struct task_struct *k) { struct kthread *kthread; @@ -272,8 +278,8 @@ static int kthread(void *_create) struct kthread *self; int ret; - self = kzalloc(sizeof(*self), GFP_KERNEL); - set_kthread_struct(self); + set_kthread_struct(current); + self = to_kthread(current); /* If user was SIGKILLed, I release the structure. */ done = xchg(&create->done, NULL); diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 4a0668acd876..5bb720829d93 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -7440,12 +7440,25 @@ void init_idle(struct task_struct *idle, int cpu) __sched_fork(0, idle); + /* + * The idle task doesn't need the kthread struct to function, but it + * is dressed up as a per-CPU kthread and thus needs to play the part + * if we want to avoid special-casing it in code that deals with per-CPU + * kthreads. + */ + set_kthread_struct(idle); + raw_spin_lock_irqsave(&idle->pi_lock, flags); raw_spin_lock(&rq->lock); idle->state = TASK_RUNNING; idle->se.exec_start = sched_clock(); - idle->flags |= PF_IDLE; + /* + * PF_KTHREAD should already be set at this point; regardless, make it + * look like a proper per-CPU kthread. + */ + idle->flags |= PF_IDLE | PF_KTHREAD | PF_NO_SETAFFINITY; + kthread_set_per_cpu(idle, cpu); scs_task_reset(idle); kasan_unpoison_task_stack(idle); @@ -7662,12 +7675,8 @@ static void balance_push(struct rq *rq) /* * Both the cpu-hotplug and stop task are in this case and are * required to complete the hotplug process. - * - * XXX: the idle task does not match kthread_is_per_cpu() due to - * histerical raisins. */ - if (rq->idle == push_task || - kthread_is_per_cpu(push_task) || + if (kthread_is_per_cpu(push_task) || is_migration_disabled(push_task)) { /* -- 2.25.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] sched: Make the idle task quack like a per-CPU kthread 2021-05-10 15:10 ` [PATCH 1/2] sched: Make the idle task quack like a per-CPU kthread Valentin Schneider @ 2021-05-10 15:57 ` Valentin Schneider 2021-05-11 7:32 ` Ingo Molnar 2021-05-19 8:09 ` [tip: sched/core] " tip-bot2 for Valentin Schneider 1 sibling, 1 reply; 13+ messages in thread From: Valentin Schneider @ 2021-05-10 15:57 UTC (permalink / raw) To: linux-kernel; +Cc: mingo, peterz, tglx, bristot, yejune.deng On 10/05/21 16:10, Valentin Schneider wrote: > This requires some extra iffery as init_idle() > call be called more than once on the same idle task. > While I'm at it, do we actually still need to suffer through this? AFAICT the extra calls are due to idle_thread_get() (used in cpuhp) calling init_idle(). However it looks to me that since 3bb5d2ee396a ("smp, idle: Allocate idle thread for each possible cpu during boot") we don't need to do that: we already have a for_each_possible_cpu(cpu) init_idle(cpu) issued at init. So can't we "simply" rely on that init-time creation, given it's done against the possible mask? I think the only thing that might need doing at later hotplug is making sure the preempt count is right (secondary startups seem to all prepare the idle task by issuing a preempt_disable()). ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] sched: Make the idle task quack like a per-CPU kthread 2021-05-10 15:57 ` Valentin Schneider @ 2021-05-11 7:32 ` Ingo Molnar 2021-05-11 9:33 ` Valentin Schneider 0 siblings, 1 reply; 13+ messages in thread From: Ingo Molnar @ 2021-05-11 7:32 UTC (permalink / raw) To: Valentin Schneider; +Cc: linux-kernel, peterz, tglx, bristot, yejune.deng * Valentin Schneider <valentin.schneider@arm.com> wrote: > On 10/05/21 16:10, Valentin Schneider wrote: > > This requires some extra iffery as init_idle() > > call be called more than once on the same idle task. > > > > While I'm at it, do we actually still need to suffer through this? No. > AFAICT the extra calls are due to idle_thread_get() (used in cpuhp) > calling init_idle(). However it looks to me that since > > 3bb5d2ee396a ("smp, idle: Allocate idle thread for each possible cpu during boot") > > we don't need to do that: we already have a > > for_each_possible_cpu(cpu) > init_idle(cpu) > > issued at init. So can't we "simply" rely on that init-time creation, > given it's done against the possible mask? I think the only thing that > might need doing at later hotplug is making sure the preempt count is > right (secondary startups seem to all prepare the idle task by issuing a > preempt_disable()). Best-case it works, worst-case we discover an unclean assumption in the init sequence and it works after we fix that. Win-win. :-) Thanks, Ingo ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] sched: Make the idle task quack like a per-CPU kthread 2021-05-11 7:32 ` Ingo Molnar @ 2021-05-11 9:33 ` Valentin Schneider 0 siblings, 0 replies; 13+ messages in thread From: Valentin Schneider @ 2021-05-11 9:33 UTC (permalink / raw) To: Ingo Molnar; +Cc: linux-kernel, peterz, tglx, bristot, yejune.deng On 11/05/21 09:32, Ingo Molnar wrote: > * Valentin Schneider <valentin.schneider@arm.com> wrote: >> AFAICT the extra calls are due to idle_thread_get() (used in cpuhp) >> calling init_idle(). However it looks to me that since >> >> 3bb5d2ee396a ("smp, idle: Allocate idle thread for each possible cpu during boot") >> >> we don't need to do that: we already have a >> >> for_each_possible_cpu(cpu) >> init_idle(cpu) >> >> issued at init. So can't we "simply" rely on that init-time creation, >> given it's done against the possible mask? I think the only thing that >> might need doing at later hotplug is making sure the preempt count is >> right (secondary startups seem to all prepare the idle task by issuing a >> preempt_disable()). > > Best-case it works, worst-case we discover an unclean assumption in the > init sequence and it works after we fix that. > > Win-win. :-) > Well I got something that seems to work, let me it test it some more and convince myself it isn't completely bonkers and I'll toss it out. > Thanks, > > Ingo ^ permalink raw reply [flat|nested] 13+ messages in thread
* [tip: sched/core] sched: Make the idle task quack like a per-CPU kthread 2021-05-10 15:10 ` [PATCH 1/2] sched: Make the idle task quack like a per-CPU kthread Valentin Schneider 2021-05-10 15:57 ` Valentin Schneider @ 2021-05-19 8:09 ` tip-bot2 for Valentin Schneider 1 sibling, 0 replies; 13+ messages in thread From: tip-bot2 for Valentin Schneider @ 2021-05-19 8:09 UTC (permalink / raw) To: linux-tip-commits Cc: Valentin Schneider, Peter Zijlstra (Intel), x86, linux-kernel The following commit has been merged into the sched/core branch of tip: Commit-ID: 00b89fe0197f0c55a045775c11553c0cdb7082fe Gitweb: https://git.kernel.org/tip/00b89fe0197f0c55a045775c11553c0cdb7082fe Author: Valentin Schneider <valentin.schneider@arm.com> AuthorDate: Mon, 10 May 2021 16:10:23 +01:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Tue, 18 May 2021 12:53:53 +02:00 sched: Make the idle task quack like a per-CPU kthread For all intents and purposes, the idle task is a per-CPU kthread. It isn't created via the same route as other pcpu kthreads however, and as a result it is missing a few bells and whistles: it fails kthread_is_per_cpu() and it doesn't have PF_NO_SETAFFINITY set. Fix the former by giving the idle task a kthread struct along with the KTHREAD_IS_PER_CPU flag. This requires some extra iffery as init_idle() call be called more than once on the same idle task. Signed-off-by: Valentin Schneider <valentin.schneider@arm.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20210510151024.2448573-2-valentin.schneider@arm.com --- include/linux/kthread.h | 2 ++ kernel/kthread.c | 30 ++++++++++++++++++------------ kernel/sched/core.c | 21 +++++++++++++++------ 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/include/linux/kthread.h b/include/linux/kthread.h index 2484ed9..d9133d6 100644 --- a/include/linux/kthread.h +++ b/include/linux/kthread.h @@ -33,6 +33,8 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data), unsigned int cpu, const char *namefmt); +void set_kthread_struct(struct task_struct *p); + void kthread_set_per_cpu(struct task_struct *k, int cpu); bool kthread_is_per_cpu(struct task_struct *k); diff --git a/kernel/kthread.c b/kernel/kthread.c index fe3f2a4..3d32683 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -68,16 +68,6 @@ enum KTHREAD_BITS { KTHREAD_SHOULD_PARK, }; -static inline void set_kthread_struct(void *kthread) -{ - /* - * We abuse ->set_child_tid to avoid the new member and because it - * can't be wrongly copied by copy_process(). We also rely on fact - * that the caller can't exec, so PF_KTHREAD can't be cleared. - */ - current->set_child_tid = (__force void __user *)kthread; -} - static inline struct kthread *to_kthread(struct task_struct *k) { WARN_ON(!(k->flags & PF_KTHREAD)); @@ -103,6 +93,22 @@ static inline struct kthread *__to_kthread(struct task_struct *p) return kthread; } +void set_kthread_struct(struct task_struct *p) +{ + struct kthread *kthread; + + if (__to_kthread(p)) + return; + + kthread = kzalloc(sizeof(*kthread), GFP_KERNEL); + /* + * We abuse ->set_child_tid to avoid the new member and because it + * can't be wrongly copied by copy_process(). We also rely on fact + * that the caller can't exec, so PF_KTHREAD can't be cleared. + */ + p->set_child_tid = (__force void __user *)kthread; +} + void free_kthread_struct(struct task_struct *k) { struct kthread *kthread; @@ -272,8 +278,8 @@ static int kthread(void *_create) struct kthread *self; int ret; - self = kzalloc(sizeof(*self), GFP_KERNEL); - set_kthread_struct(self); + set_kthread_struct(current); + self = to_kthread(current); /* If user was SIGKILLed, I release the structure. */ done = xchg(&create->done, NULL); diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 24fd795..6a5124c 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -8234,12 +8234,25 @@ void __init init_idle(struct task_struct *idle, int cpu) __sched_fork(0, idle); + /* + * The idle task doesn't need the kthread struct to function, but it + * is dressed up as a per-CPU kthread and thus needs to play the part + * if we want to avoid special-casing it in code that deals with per-CPU + * kthreads. + */ + set_kthread_struct(idle); + raw_spin_lock_irqsave(&idle->pi_lock, flags); raw_spin_rq_lock(rq); idle->state = TASK_RUNNING; idle->se.exec_start = sched_clock(); - idle->flags |= PF_IDLE; + /* + * PF_KTHREAD should already be set at this point; regardless, make it + * look like a proper per-CPU kthread. + */ + idle->flags |= PF_IDLE | PF_KTHREAD | PF_NO_SETAFFINITY; + kthread_set_per_cpu(idle, cpu); scs_task_reset(idle); kasan_unpoison_task_stack(idle); @@ -8456,12 +8469,8 @@ static void balance_push(struct rq *rq) /* * Both the cpu-hotplug and stop task are in this case and are * required to complete the hotplug process. - * - * XXX: the idle task does not match kthread_is_per_cpu() due to - * histerical raisins. */ - if (rq->idle == push_task || - kthread_is_per_cpu(push_task) || + if (kthread_is_per_cpu(push_task) || is_migration_disabled(push_task)) { /* ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed 2021-05-10 15:10 [PATCH 0/2] sched: Address idle task vs pcpu kthread checks Valentin Schneider 2021-05-10 15:10 ` [PATCH 1/2] sched: Make the idle task quack like a per-CPU kthread Valentin Schneider @ 2021-05-10 15:10 ` Valentin Schneider 2021-05-19 8:09 ` [tip: sched/core] " tip-bot2 for Yejune Deng 2021-05-19 9:02 ` tip-bot2 for Yejune Deng 2021-05-12 11:00 ` [PATCH 0/2] sched: Address idle task vs pcpu kthread checks Peter Zijlstra 2 siblings, 2 replies; 13+ messages in thread From: Valentin Schneider @ 2021-05-10 15:10 UTC (permalink / raw) To: linux-kernel; +Cc: Yejune Deng, mingo, peterz, tglx, bristot From: Yejune Deng <yejune.deng@gmail.com> is_percpu_thread() more elegantly handles SMP vs UP, and further checks the presence of PF_NO_SETAFFINITY. This lets us catch cases where check_preemption_disabled() can race with a concurrent sched_setaffinity(). Signed-off-by: Yejune Deng <yejune.deng@gmail.com> [Amended changelog] Signed-off-by: Valentin Schneider <valentin.schneider@arm.com> --- lib/smp_processor_id.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/smp_processor_id.c b/lib/smp_processor_id.c index 1c1dbd300325..046ac6297c78 100644 --- a/lib/smp_processor_id.c +++ b/lib/smp_processor_id.c @@ -19,11 +19,7 @@ unsigned int check_preemption_disabled(const char *what1, const char *what2) if (irqs_disabled()) goto out; - /* - * Kernel threads bound to a single CPU can safely use - * smp_processor_id(): - */ - if (current->nr_cpus_allowed == 1) + if (is_percpu_thread()) goto out; #ifdef CONFIG_SMP -- 2.25.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [tip: sched/core] lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed 2021-05-10 15:10 ` [PATCH 2/2] lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed Valentin Schneider @ 2021-05-19 8:09 ` tip-bot2 for Yejune Deng 2021-05-19 9:02 ` tip-bot2 for Yejune Deng 1 sibling, 0 replies; 13+ messages in thread From: tip-bot2 for Yejune Deng @ 2021-05-19 8:09 UTC (permalink / raw) To: linux-tip-commits Cc: Valentin Schneider, Yejune Deng, Peter Zijlstra (Intel), x86, linux-kernel The following commit has been merged into the sched/core branch of tip: Commit-ID: 0019699518cc026b5bd912425be8e424843d5b33 Gitweb: https://git.kernel.org/tip/0019699518cc026b5bd912425be8e424843d5b33 Author: Yejune Deng <yejune.deng@gmail.com> AuthorDate: Mon, 10 May 2021 16:10:24 +01:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Tue, 18 May 2021 12:53:54 +02:00 lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed is_percpu_thread() more elegantly handles SMP vs UP, and further checks the presence of PF_NO_SETAFFINITY. This lets us catch cases where check_preemption_disabled() can race with a concurrent sched_setaffinity(). [Amended changelog] Signed-off-by: Valentin Schneider <valentin.schneider@arm.com> Signed-off-by: Yejune Deng <yejune.deng@gmail.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20210510151024.2448573-3-valentin.schneider@arm.com --- lib/smp_processor_id.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/smp_processor_id.c b/lib/smp_processor_id.c index 1c1dbd3..046ac62 100644 --- a/lib/smp_processor_id.c +++ b/lib/smp_processor_id.c @@ -19,11 +19,7 @@ unsigned int check_preemption_disabled(const char *what1, const char *what2) if (irqs_disabled()) goto out; - /* - * Kernel threads bound to a single CPU can safely use - * smp_processor_id(): - */ - if (current->nr_cpus_allowed == 1) + if (is_percpu_thread()) goto out; #ifdef CONFIG_SMP ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [tip: sched/core] lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed 2021-05-10 15:10 ` [PATCH 2/2] lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed Valentin Schneider 2021-05-19 8:09 ` [tip: sched/core] " tip-bot2 for Yejune Deng @ 2021-05-19 9:02 ` tip-bot2 for Yejune Deng 2021-05-31 10:21 ` [PATCH] sched,init: Fix DEBUG_PREEMPT vs early boot Peter Zijlstra 1 sibling, 1 reply; 13+ messages in thread From: tip-bot2 for Yejune Deng @ 2021-05-19 9:02 UTC (permalink / raw) To: linux-tip-commits Cc: Yejune Deng, Valentin Schneider, Peter Zijlstra (Intel), x86, linux-kernel The following commit has been merged into the sched/core branch of tip: Commit-ID: 570a752b7a9bd03b50ad6420cd7f10092cc11bd3 Gitweb: https://git.kernel.org/tip/570a752b7a9bd03b50ad6420cd7f10092cc11bd3 Author: Yejune Deng <yejune.deng@gmail.com> AuthorDate: Mon, 10 May 2021 16:10:24 +01:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Wed, 19 May 2021 10:51:40 +02:00 lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed is_percpu_thread() more elegantly handles SMP vs UP, and further checks the presence of PF_NO_SETAFFINITY. This lets us catch cases where check_preemption_disabled() can race with a concurrent sched_setaffinity(). Signed-off-by: Yejune Deng <yejune.deng@gmail.com> [Amended changelog] Signed-off-by: Valentin Schneider <valentin.schneider@arm.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20210510151024.2448573-3-valentin.schneider@arm.com --- lib/smp_processor_id.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/smp_processor_id.c b/lib/smp_processor_id.c index 1c1dbd3..046ac62 100644 --- a/lib/smp_processor_id.c +++ b/lib/smp_processor_id.c @@ -19,11 +19,7 @@ unsigned int check_preemption_disabled(const char *what1, const char *what2) if (irqs_disabled()) goto out; - /* - * Kernel threads bound to a single CPU can safely use - * smp_processor_id(): - */ - if (current->nr_cpus_allowed == 1) + if (is_percpu_thread()) goto out; #ifdef CONFIG_SMP ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] sched,init: Fix DEBUG_PREEMPT vs early boot 2021-05-19 9:02 ` tip-bot2 for Yejune Deng @ 2021-05-31 10:21 ` Peter Zijlstra 2021-06-01 11:54 ` Valentin Schneider 2021-06-01 14:04 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra 0 siblings, 2 replies; 13+ messages in thread From: Peter Zijlstra @ 2021-05-31 10:21 UTC (permalink / raw) To: linux-kernel; +Cc: linux-tip-commits, Yejune Deng, Valentin Schneider, x86 On Wed, May 19, 2021 at 09:02:34AM -0000, tip-bot2 for Yejune Deng wrote: > The following commit has been merged into the sched/core branch of tip: > > Commit-ID: 570a752b7a9bd03b50ad6420cd7f10092cc11bd3 > Gitweb: https://git.kernel.org/tip/570a752b7a9bd03b50ad6420cd7f10092cc11bd3 > Author: Yejune Deng <yejune.deng@gmail.com> > AuthorDate: Mon, 10 May 2021 16:10:24 +01:00 > Committer: Peter Zijlstra <peterz@infradead.org> > CommitterDate: Wed, 19 May 2021 10:51:40 +02:00 > > lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed > > is_percpu_thread() more elegantly handles SMP vs UP, and further checks the > presence of PF_NO_SETAFFINITY. This lets us catch cases where > check_preemption_disabled() can race with a concurrent sched_setaffinity(). > > Signed-off-by: Yejune Deng <yejune.deng@gmail.com> > [Amended changelog] > Signed-off-by: Valentin Schneider <valentin.schneider@arm.com> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Link: https://lkml.kernel.org/r/20210510151024.2448573-3-valentin.schneider@arm.com > --- > lib/smp_processor_id.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/lib/smp_processor_id.c b/lib/smp_processor_id.c > index 1c1dbd3..046ac62 100644 > --- a/lib/smp_processor_id.c > +++ b/lib/smp_processor_id.c > @@ -19,11 +19,7 @@ unsigned int check_preemption_disabled(const char *what1, const char *what2) > if (irqs_disabled()) > goto out; > > - /* > - * Kernel threads bound to a single CPU can safely use > - * smp_processor_id(): > - */ > - if (current->nr_cpus_allowed == 1) > + if (is_percpu_thread()) > goto out; So my test box was unhappy with all this and started spewing lots of DEBUG_PREEMPT warns on boot. This extends 8fb12156b8db6 to cover the new requirement. --- Subject: sched,init: Fix DEBUG_PREEMPT vs early boot Extend 8fb12156b8db ("init: Pin init task to the boot CPU, initially") to cover the new PF_NO_SETAFFINITY requirement. While there, move wait_for_completion(&kthreadd_done) into kernel_init() to make it absolutely clear it is the very first thing done by the init thread. Fixes: 570a752b7a9b ("lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed") Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- init/main.c | 11 ++++++----- kernel/sched/core.c | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/init/main.c b/init/main.c index 7b027d9c5c89..e945ec82b8a5 100644 --- a/init/main.c +++ b/init/main.c @@ -692,6 +692,7 @@ noinline void __ref rest_init(void) */ rcu_read_lock(); tsk = find_task_by_pid_ns(pid, &init_pid_ns); + tsk->flags |= PF_NO_SETAFFINITY; set_cpus_allowed_ptr(tsk, cpumask_of(smp_processor_id())); rcu_read_unlock(); @@ -1440,6 +1441,11 @@ static int __ref kernel_init(void *unused) { int ret; + /* + * Wait until kthreadd is all set-up. + */ + wait_for_completion(&kthreadd_done); + kernel_init_freeable(); /* need to finish all async __init code before freeing the memory */ async_synchronize_full(); @@ -1520,11 +1526,6 @@ void __init console_on_rootfs(void) static noinline void __init kernel_init_freeable(void) { - /* - * Wait until kthreadd is all set-up. - */ - wait_for_completion(&kthreadd_done); - /* Now the scheduler is fully set up and can do blocking allocations */ gfp_allowed_mask = __GFP_BITS_MASK; diff --git a/kernel/sched/core.c b/kernel/sched/core.c index adea0b1e8036..ae7737e6c2b2 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -8867,6 +8867,7 @@ void __init sched_init_smp(void) /* Move init over to a non-isolated CPU */ if (set_cpus_allowed_ptr(current, housekeeping_cpumask(HK_FLAG_DOMAIN)) < 0) BUG(); + current->flags &= ~PF_NO_SETAFFINITY; sched_init_granularity(); init_sched_rt_class(); ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] sched,init: Fix DEBUG_PREEMPT vs early boot 2021-05-31 10:21 ` [PATCH] sched,init: Fix DEBUG_PREEMPT vs early boot Peter Zijlstra @ 2021-06-01 11:54 ` Valentin Schneider 2021-06-01 14:04 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra 1 sibling, 0 replies; 13+ messages in thread From: Valentin Schneider @ 2021-06-01 11:54 UTC (permalink / raw) To: Peter Zijlstra, linux-kernel; +Cc: linux-tip-commits, Yejune Deng, x86 On 31/05/21 12:21, Peter Zijlstra wrote: > On Wed, May 19, 2021 at 09:02:34AM -0000, tip-bot2 for Yejune Deng wrote: >> @@ -19,11 +19,7 @@ unsigned int check_preemption_disabled(const char *what1, const char *what2) >> if (irqs_disabled()) >> goto out; >> >> - /* >> - * Kernel threads bound to a single CPU can safely use >> - * smp_processor_id(): >> - */ >> - if (current->nr_cpus_allowed == 1) >> + if (is_percpu_thread()) >> goto out; > > So my test box was unhappy with all this and started spewing lots of > DEBUG_PREEMPT warns on boot. > I get these too, though can't recall getting them when testing the above. I think it's tied with what Frederic found out with copy_process() copying PF_NO_SETAFFINITY, which it now no longer does. > This extends 8fb12156b8db6 to cover the new requirement. > > --- > Subject: sched,init: Fix DEBUG_PREEMPT vs early boot > > Extend 8fb12156b8db ("init: Pin init task to the boot CPU, initially") > to cover the new PF_NO_SETAFFINITY requirement. > > While there, move wait_for_completion(&kthreadd_done) into kernel_init() > to make it absolutely clear it is the very first thing done by the init > thread. > > Fixes: 570a752b7a9b ("lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed") > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Tested-by: Valentin Schneider <valentin.schneider@arm.com> Reviewed-by: Valentin Schneider <valentin.schneider@arm.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [tip: sched/core] sched,init: Fix DEBUG_PREEMPT vs early boot 2021-05-31 10:21 ` [PATCH] sched,init: Fix DEBUG_PREEMPT vs early boot Peter Zijlstra 2021-06-01 11:54 ` Valentin Schneider @ 2021-06-01 14:04 ` tip-bot2 for Peter Zijlstra 1 sibling, 0 replies; 13+ messages in thread From: tip-bot2 for Peter Zijlstra @ 2021-06-01 14:04 UTC (permalink / raw) To: linux-tip-commits Cc: Peter Zijlstra (Intel), Valentin Schneider, Borislav Petkov, x86, linux-kernel The following commit has been merged into the sched/core branch of tip: Commit-ID: 15faafc6b449777a85c0cf82dd8286c293fed4eb Gitweb: https://git.kernel.org/tip/15faafc6b449777a85c0cf82dd8286c293fed4eb Author: Peter Zijlstra <peterz@infradead.org> AuthorDate: Mon, 31 May 2021 12:21:13 +02:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Tue, 01 Jun 2021 16:00:11 +02:00 sched,init: Fix DEBUG_PREEMPT vs early boot Extend 8fb12156b8db ("init: Pin init task to the boot CPU, initially") to cover the new PF_NO_SETAFFINITY requirement. While there, move wait_for_completion(&kthreadd_done) into kernel_init() to make it absolutely clear it is the very first thing done by the init thread. Fixes: 570a752b7a9b ("lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed") Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Valentin Schneider <valentin.schneider@arm.com> Tested-by: Valentin Schneider <valentin.schneider@arm.com> Tested-by: Borislav Petkov <bp@alien8.de> Link: https://lkml.kernel.org/r/YLS4mbKUrA3Gnb4t@hirez.programming.kicks-ass.net --- init/main.c | 11 ++++++----- kernel/sched/core.c | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/init/main.c b/init/main.c index 7b027d9..e945ec8 100644 --- a/init/main.c +++ b/init/main.c @@ -692,6 +692,7 @@ noinline void __ref rest_init(void) */ rcu_read_lock(); tsk = find_task_by_pid_ns(pid, &init_pid_ns); + tsk->flags |= PF_NO_SETAFFINITY; set_cpus_allowed_ptr(tsk, cpumask_of(smp_processor_id())); rcu_read_unlock(); @@ -1440,6 +1441,11 @@ static int __ref kernel_init(void *unused) { int ret; + /* + * Wait until kthreadd is all set-up. + */ + wait_for_completion(&kthreadd_done); + kernel_init_freeable(); /* need to finish all async __init code before freeing the memory */ async_synchronize_full(); @@ -1520,11 +1526,6 @@ void __init console_on_rootfs(void) static noinline void __init kernel_init_freeable(void) { - /* - * Wait until kthreadd is all set-up. - */ - wait_for_completion(&kthreadd_done); - /* Now the scheduler is fully set up and can do blocking allocations */ gfp_allowed_mask = __GFP_BITS_MASK; diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 3d25272..e205c19 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -8862,6 +8862,7 @@ void __init sched_init_smp(void) /* Move init over to a non-isolated CPU */ if (set_cpus_allowed_ptr(current, housekeeping_cpumask(HK_FLAG_DOMAIN)) < 0) BUG(); + current->flags &= ~PF_NO_SETAFFINITY; sched_init_granularity(); init_sched_rt_class(); ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] sched: Address idle task vs pcpu kthread checks 2021-05-10 15:10 [PATCH 0/2] sched: Address idle task vs pcpu kthread checks Valentin Schneider 2021-05-10 15:10 ` [PATCH 1/2] sched: Make the idle task quack like a per-CPU kthread Valentin Schneider 2021-05-10 15:10 ` [PATCH 2/2] lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed Valentin Schneider @ 2021-05-12 11:00 ` Peter Zijlstra 2 siblings, 0 replies; 13+ messages in thread From: Peter Zijlstra @ 2021-05-12 11:00 UTC (permalink / raw) To: Valentin Schneider; +Cc: linux-kernel, mingo, tglx, bristot, yejune.deng On Mon, May 10, 2021 at 04:10:22PM +0100, Valentin Schneider wrote: > Note: I remember seeing some patch(es) from Peter tackling this exact > problem, but I couldn't find them again. Found it (by accident), yours is nicer though :-) > Valentin Schneider (1): > sched: Make the idle task quack like a per-CPU kthread > > Yejune Deng (1): > lib/smp_processor_id: Use is_percpu_thread() instead of > nr_cpus_allowed > > include/linux/kthread.h | 2 ++ > kernel/kthread.c | 30 ++++++++++++++++++------------ > kernel/sched/core.c | 21 +++++++++++++++------ > lib/smp_processor_id.c | 6 +----- > 4 files changed, 36 insertions(+), 23 deletions(-) Thanks! ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-06-01 14:05 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-10 15:10 [PATCH 0/2] sched: Address idle task vs pcpu kthread checks Valentin Schneider 2021-05-10 15:10 ` [PATCH 1/2] sched: Make the idle task quack like a per-CPU kthread Valentin Schneider 2021-05-10 15:57 ` Valentin Schneider 2021-05-11 7:32 ` Ingo Molnar 2021-05-11 9:33 ` Valentin Schneider 2021-05-19 8:09 ` [tip: sched/core] " tip-bot2 for Valentin Schneider 2021-05-10 15:10 ` [PATCH 2/2] lib/smp_processor_id: Use is_percpu_thread() instead of nr_cpus_allowed Valentin Schneider 2021-05-19 8:09 ` [tip: sched/core] " tip-bot2 for Yejune Deng 2021-05-19 9:02 ` tip-bot2 for Yejune Deng 2021-05-31 10:21 ` [PATCH] sched,init: Fix DEBUG_PREEMPT vs early boot Peter Zijlstra 2021-06-01 11:54 ` Valentin Schneider 2021-06-01 14:04 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra 2021-05-12 11:00 ` [PATCH 0/2] sched: Address idle task vs pcpu kthread checks Peter Zijlstra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).