All of lore.kernel.org
 help / color / mirror / Atom feed
* [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	[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	[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

* 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

* [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	[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	[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	[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	[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	[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.