* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.