Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1642752 > unrolled thread
| Started by | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| First post | 2017-05-16 21:00 +0200 |
| Last post | 2017-05-23 11:10 +0200 |
| Articles | 6 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[patch V2 17/17] sched: Enable might_sleep() and smp_processor_id() checks early Thomas Gleixner <tglx@linutronix.de> - 2017-05-16 21:00 +0200
Re: [patch V2 17/17] sched: Enable might_sleep() and smp_processor_id() checks early Steven Rostedt <rostedt@goodmis.org> - 2017-05-16 21:20 +0200
Re: [patch V2 17/17] sched: Enable might_sleep() and smp_processor_id() checks early Thomas Gleixner <tglx@linutronix.de> - 2017-05-17 00:50 +0200
Re: [patch V2 17/17] sched: Enable might_sleep() and smp_processor_id() checks early Steven Rostedt <rostedt@goodmis.org> - 2017-05-17 01:00 +0200
Re: [patch V2 17/17] sched: Enable might_sleep() and smp_processor_id() checks early Mark Rutland <mark.rutland@arm.com> - 2017-05-17 13:10 +0200
[tip:sched/core] sched/core: Enable might_sleep() and smp_processor_id() checks early tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-05-23 11:10 +0200
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2017-05-16 21:00 +0200 |
| Subject | [patch V2 17/17] sched: Enable might_sleep() and smp_processor_id() checks early |
| Message-ID | <tHPRo-M2-39@gated-at.bofh.it> |
might_sleep() and smp_processor_id() checks are enabled after the boot process is done. That hides bugs in the smp bringup and driver initialization code. Enable it right when the scheduler starts working, i.e. when init task and kthreadd have been created and right before the idle task enables preemption. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- init/main.c | 10 ++++++++++ kernel/sched/core.c | 4 +++- lib/smp_processor_id.c | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) --- a/init/main.c +++ b/init/main.c @@ -414,6 +414,16 @@ static noinline void __ref rest_init(voi rcu_read_lock(); kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns); rcu_read_unlock(); + + /* + * Enable might_sleep() and smp_processor_id() checks. + * They cannot be enabled earlier because with CONFIG_PRREMPT=y + * kernel_thread() would trigger might_sleep() splats. With + * CONFIG_PREEMPT_VOLUNTARY=y the init task might have scheduled + * already, but it's stuck on the kthreadd_done completion. + */ + system_state = SYSTEM_SCHEDULING; + complete(&kthreadd_done); /* --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -6226,8 +6226,10 @@ void ___might_sleep(const char *file, in if ((preempt_count_equals(preempt_offset) && !irqs_disabled() && !is_idle_task(current)) || - system_state != SYSTEM_RUNNING || oops_in_progress) + system_state == SYSTEM_BOOTING || system_state > SYSTEM_RUNNING || + oops_in_progress) return; + if (time_before(jiffies, prev_jiffy + HZ) && prev_jiffy) return; prev_jiffy = jiffies; --- a/lib/smp_processor_id.c +++ b/lib/smp_processor_id.c @@ -28,7 +28,7 @@ notrace static unsigned int check_preemp /* * It is valid to assume CPU-locality during early bootup: */ - if (system_state != SYSTEM_RUNNING) + if (system_state < SYSTEM_SCHEDULING) goto out; /*
[toc] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-05-16 21:20 +0200 |
| Subject | Re: [patch V2 17/17] sched: Enable might_sleep() and smp_processor_id() checks early |
| Message-ID | <tHQaJ-18a-5@gated-at.bofh.it> |
| In reply to | #1642752 |
On Tue, 16 May 2017 20:42:48 +0200 Thomas Gleixner <tglx@linutronix.de> wrote: > might_sleep() and smp_processor_id() checks are enabled after the boot > process is done. That hides bugs in the smp bringup and driver > initialization code. > > Enable it right when the scheduler starts working, i.e. when init task and > kthreadd have been created and right before the idle task enables > preemption. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > init/main.c | 10 ++++++++++ > kernel/sched/core.c | 4 +++- > lib/smp_processor_id.c | 2 +- > 3 files changed, 14 insertions(+), 2 deletions(-) > > --- a/init/main.c > +++ b/init/main.c > @@ -414,6 +414,16 @@ static noinline void __ref rest_init(voi > rcu_read_lock(); > kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns); > rcu_read_unlock(); > + > + /* > + * Enable might_sleep() and smp_processor_id() checks. > + * They cannot be enabled earlier because with CONFIG_PRREMPT=y My cat's version of CONFIG_PREEMPT, it's CONFIG_ PRR EMPT! > + * kernel_thread() would trigger might_sleep() splats. With > + * CONFIG_PREEMPT_VOLUNTARY=y the init task might have scheduled > + * already, but it's stuck on the kthreadd_done completion. > + */ > + system_state = SYSTEM_SCHEDULING; > + > complete(&kthreadd_done); > > /* > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -6226,8 +6226,10 @@ void ___might_sleep(const char *file, in > > if ((preempt_count_equals(preempt_offset) && !irqs_disabled() && > !is_idle_task(current)) || > - system_state != SYSTEM_RUNNING || oops_in_progress) > + system_state == SYSTEM_BOOTING || system_state > SYSTEM_RUNNING || > + oops_in_progress) > return; > + > if (time_before(jiffies, prev_jiffy + HZ) && prev_jiffy) > return; > prev_jiffy = jiffies; > --- a/lib/smp_processor_id.c > +++ b/lib/smp_processor_id.c > @@ -28,7 +28,7 @@ notrace static unsigned int check_preemp > /* > * It is valid to assume CPU-locality during early bootup: > */ > - if (system_state != SYSTEM_RUNNING) > + if (system_state < SYSTEM_SCHEDULING) Do we want to ignore halting or rebooting too? -- Steve > goto out; > > /* >
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2017-05-17 00:50 +0200 |
| Message-ID | <tHTrX-3gi-1@gated-at.bofh.it> |
| In reply to | #1642774 |
On Tue, 16 May 2017, Steven Rostedt wrote: > On Tue, 16 May 2017 20:42:48 +0200 > Thomas Gleixner <tglx@linutronix.de> wrote: > > + > > + /* > > + * Enable might_sleep() and smp_processor_id() checks. > > + * They cannot be enabled earlier because with CONFIG_PRREMPT=y > > My cat's version of CONFIG_PREEMPT, it's CONFIG_ PRR EMPT! I don't have a cat and I don't need one for creating typos. I'm perfectly able to do that myself :) > > + * kernel_thread() would trigger might_sleep() splats. With > > + * CONFIG_PREEMPT_VOLUNTARY=y the init task might have scheduled > > + * already, but it's stuck on the kthreadd_done completion. > > + */ > > + system_state = SYSTEM_SCHEDULING; > > + > > complete(&kthreadd_done); > > > > /* > > --- a/kernel/sched/core.c > > +++ b/kernel/sched/core.c > > @@ -6226,8 +6226,10 @@ void ___might_sleep(const char *file, in > > > > if ((preempt_count_equals(preempt_offset) && !irqs_disabled() && > > !is_idle_task(current)) || > > - system_state != SYSTEM_RUNNING || oops_in_progress) > > + system_state == SYSTEM_BOOTING || system_state > SYSTEM_RUNNING || > > + oops_in_progress) > > return; > > + > > if (time_before(jiffies, prev_jiffy + HZ) && prev_jiffy) > > return; > > prev_jiffy = jiffies; > > --- a/lib/smp_processor_id.c > > +++ b/lib/smp_processor_id.c > > @@ -28,7 +28,7 @@ notrace static unsigned int check_preemp > > /* > > * It is valid to assume CPU-locality during early bootup: > > */ > > - if (system_state != SYSTEM_RUNNING) > > + if (system_state < SYSTEM_SCHEDULING) > > Do we want to ignore halting or rebooting too? I don't think so. After setting those states, interesting stuff like device_shutdown() gets invoked. We want the coverage there. Thanks, tglx
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-05-17 01:00 +0200 |
| Subject | Re: [patch V2 17/17] sched: Enable might_sleep() and smp_processor_id() checks early |
| Message-ID | <tHTBD-3kx-3@gated-at.bofh.it> |
| In reply to | #1642884 |
On Wed, 17 May 2017 00:46:37 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote: > > > --- a/lib/smp_processor_id.c > > > +++ b/lib/smp_processor_id.c > > > @@ -28,7 +28,7 @@ notrace static unsigned int check_preemp > > > /* > > > * It is valid to assume CPU-locality during early bootup: > > > */ > > > - if (system_state != SYSTEM_RUNNING) > > > + if (system_state < SYSTEM_SCHEDULING) > > > > Do we want to ignore halting or rebooting too? > > I don't think so. After setting those states, interesting stuff like > device_shutdown() gets invoked. We want the coverage there. Then I'd suggest that you update the change log, as this patch also adds coverage to those states as well. -- Steve
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2017-05-17 13:10 +0200 |
| Subject | Re: [patch V2 17/17] sched: Enable might_sleep() and smp_processor_id() checks early |
| Message-ID | <tI506-2ji-13@gated-at.bofh.it> |
| In reply to | #1642752 |
On Tue, May 16, 2017 at 08:42:48PM +0200, Thomas Gleixner wrote: > might_sleep() and smp_processor_id() checks are enabled after the boot > process is done. That hides bugs in the smp bringup and driver > initialization code. > > Enable it right when the scheduler starts working, i.e. when init task and > kthreadd have been created and right before the idle task enables > preemption. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Modulo Steve's comments, FWIW: Acked-by: Mark Rutland <mark.rutland@arm.com> Mark. > --- > init/main.c | 10 ++++++++++ > kernel/sched/core.c | 4 +++- > lib/smp_processor_id.c | 2 +- > 3 files changed, 14 insertions(+), 2 deletions(-) > > --- a/init/main.c > +++ b/init/main.c > @@ -414,6 +414,16 @@ static noinline void __ref rest_init(voi > rcu_read_lock(); > kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns); > rcu_read_unlock(); > + > + /* > + * Enable might_sleep() and smp_processor_id() checks. > + * They cannot be enabled earlier because with CONFIG_PRREMPT=y > + * kernel_thread() would trigger might_sleep() splats. With > + * CONFIG_PREEMPT_VOLUNTARY=y the init task might have scheduled > + * already, but it's stuck on the kthreadd_done completion. > + */ > + system_state = SYSTEM_SCHEDULING; > + > complete(&kthreadd_done); > > /* > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -6226,8 +6226,10 @@ void ___might_sleep(const char *file, in > > if ((preempt_count_equals(preempt_offset) && !irqs_disabled() && > !is_idle_task(current)) || > - system_state != SYSTEM_RUNNING || oops_in_progress) > + system_state == SYSTEM_BOOTING || system_state > SYSTEM_RUNNING || > + oops_in_progress) > return; > + > if (time_before(jiffies, prev_jiffy + HZ) && prev_jiffy) > return; > prev_jiffy = jiffies; > --- a/lib/smp_processor_id.c > +++ b/lib/smp_processor_id.c > @@ -28,7 +28,7 @@ notrace static unsigned int check_preemp > /* > * It is valid to assume CPU-locality during early bootup: > */ > - if (system_state != SYSTEM_RUNNING) > + if (system_state < SYSTEM_SCHEDULING) > goto out; > > /* > >
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Thomas Gleixner <tipbot@zytor.com> |
|---|---|
| Date | 2017-05-23 11:10 +0200 |
| Subject | [tip:sched/core] sched/core: Enable might_sleep() and smp_processor_id() checks early |
| Message-ID | <tKdZg-15V-25@gated-at.bofh.it> |
| In reply to | #1642752 |
Commit-ID: 1c3c5eab171590f86edd8d31389d61dd1efe3037 Gitweb: http://git.kernel.org/tip/1c3c5eab171590f86edd8d31389d61dd1efe3037 Author: Thomas Gleixner <tglx@linutronix.de> AuthorDate: Tue, 16 May 2017 20:42:48 +0200 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 23 May 2017 10:01:38 +0200 sched/core: Enable might_sleep() and smp_processor_id() checks early might_sleep() and smp_processor_id() checks are enabled after the boot process is done. That hides bugs in the SMP bringup and driver initialization code. Enable it right when the scheduler starts working, i.e. when init task and kthreadd have been created and right before the idle task enables preemption. Tested-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Mark Rutland <mark.rutland@arm.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Steven Rostedt <rostedt@goodmis.org> Link: http://lkml.kernel.org/r/20170516184736.272225698@linutronix.de Signed-off-by: Ingo Molnar <mingo@kernel.org> --- init/main.c | 10 ++++++++++ kernel/sched/core.c | 4 +++- lib/smp_processor_id.c | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/init/main.c b/init/main.c index badae3b..df58a41 100644 --- a/init/main.c +++ b/init/main.c @@ -414,6 +414,16 @@ static noinline void __ref rest_init(void) rcu_read_lock(); kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns); rcu_read_unlock(); + + /* + * Enable might_sleep() and smp_processor_id() checks. + * They cannot be enabled earlier because with CONFIG_PRREMPT=y + * kernel_thread() would trigger might_sleep() splats. With + * CONFIG_PREEMPT_VOLUNTARY=y the init task might have scheduled + * already, but it's stuck on the kthreadd_done completion. + */ + system_state = SYSTEM_SCHEDULING; + complete(&kthreadd_done); /* diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 877241e..c3e50ca 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -6238,8 +6238,10 @@ void ___might_sleep(const char *file, int line, int preempt_offset) if ((preempt_count_equals(preempt_offset) && !irqs_disabled() && !is_idle_task(current)) || - system_state != SYSTEM_RUNNING || oops_in_progress) + system_state == SYSTEM_BOOTING || system_state > SYSTEM_RUNNING || + oops_in_progress) return; + if (time_before(jiffies, prev_jiffy + HZ) && prev_jiffy) return; prev_jiffy = jiffies; diff --git a/lib/smp_processor_id.c b/lib/smp_processor_id.c index 690d75b..2fb007b 100644 --- a/lib/smp_processor_id.c +++ b/lib/smp_processor_id.c @@ -28,7 +28,7 @@ notrace static unsigned int check_preemption_disabled(const char *what1, /* * It is valid to assume CPU-locality during early bootup: */ - if (system_state != SYSTEM_RUNNING) + if (system_state < SYSTEM_SCHEDULING) goto out; /*
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web