[PATCH 0/5] context_tracking: Flatter archs not using exception_enter/exit()

[PATCH 0/5] context_tracking: Flatter archs not using exception_enter/exit()

[Frederic Weisbecker]

In order to be able to turn on/off nohz_full at runtime, we'll need
the archs to give up saving the context tracking state on the task stack
because this forces context tracking to run system wide, even on CPUs
that don't have nohz_full enabled.

Now being able to do so require sane entry code meeting some
requirements that at least x86 just achieved recently (I haven't checked
other archs yet).

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	core/isolation

HEAD: d52271b6d5d02ead1916d65b013d11a7d90501b9

Thanks,
	Frederic
---

Frederic Weisbecker (5):
      context_tracking: Introduce HAVE_CONTEXT_TRACKING_OFFSTACK
      context_tracking:  Don't implement exception_enter/exit() on CONFIG_HAVE_CONTEXT_TRACKING_OFFSTACK
      sched: Detect call to schedule from critical entry code
      context_tracking: Only define schedule_user() on !HAVE_CONTEXT_TRACKING_OFFSTACK archs
      x86: Support HAVE_CONTEXT_TRACKING_OFFSTACK


 arch/Kconfig                     | 17 +++++++++++++++++
 arch/x86/Kconfig                 |  1 +
 include/linux/context_tracking.h |  6 ++++--
 kernel/sched/core.c              |  3 ++-
 4 files changed, 24 insertions(+), 3 deletions(-)

[PATCH 1/5] context_tracking: Introduce HAVE_CONTEXT_TRACKING_OFFSTACK

[Frederic Weisbecker]

Historically, context tracking had to deal with fragile entry code path,
ie: before user_exit() is called and after user_enter() is called, in
case some of those spots would call schedule() or use RCU. On such
cases, the site had to be protected between exception_enter() and
exception_exit() that save the context tracking state in the task stack.

Such sleepable fragile code path had many different origins: tracing,
exceptions, early or late calls to context tracking on syscalls...

Aside of that not being pretty, saving the context tracking state on
the task stack forces us to run context tracking on all CPUs, including
housekeepers, and prevents us to completely shutdown nohz_full at
runtime on a CPU in the future as context tracking and its overhead
would still need to run system wide.

Now thanks to the extensive efforts to sanitize x86 entry code, those
conditions have been removed and we can now get rid of these workarounds
in this architecture.

Create a Kconfig feature to express this achievement.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Phil Auld <pauld@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/Kconfig | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index af14a567b493..e6ec866e5648 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -572,6 +572,23 @@ config HAVE_CONTEXT_TRACKING
 	  protected inside rcu_irq_enter/rcu_irq_exit() but preemption or signal
 	  handling on irq exit still need to be protected.
 
+config HAVE_CONTEXT_TRACKING_OFFSTACK
+	bool
+	help
+	  Architecture neither relies on exception_enter()/exception_exit()
+	  nor on schedule_user(). Also preempt_schedule_notrace() and
+	  preempt_schedule_irq() can't be called in a preemptible section
+	  while context tracking is CONTEXT_USER. This feature reflects a sane
+	  entry implementation where the following requirements are met on
+	  critical entry code, ie: before user_exit() or after user_enter():
+
+	  - Critical entry code isn't preemptible (or better yet:
+	    not interruptible).
+	  - No use of RCU read side critical sections, unless rcu_nmi_enter()
+	    got called.
+	  - No use of instrumentation, unless instrumentation_begin() got
+	    called.
+
 config HAVE_TIF_NOHZ
 	bool
 	help
-- 
2.25.1

[PATCH 2/5] context_tracking: Don't implement exception_enter/exit() on CONFIG_HAVE_CONTEXT_TRACKING_OFFSTACK

[Frederic Weisbecker]

An architecture that provides this Kconfig feature doesn't need to
store the context tracking state on the task stack because its entry
code has been sanitized such that fragile path aren't preemptible
and special use of tracing and RCU read side critical sections in these
areas have been explicitly annotated.

Hence the exception_enter()/exception_exit() couple doesn't need to be
implemented in this case.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Phil Auld <pauld@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/context_tracking.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
index d53cd331c4dd..bceb06498521 100644
--- a/include/linux/context_tracking.h
+++ b/include/linux/context_tracking.h
@@ -51,7 +51,8 @@ static inline enum ctx_state exception_enter(void)
 {
 	enum ctx_state prev_ctx;
 
-	if (!context_tracking_enabled())
+	if (IS_ENABLED(CONFIG_HAVE_CONTEXT_TRACKING_OFFSTACK) ||
+	    !context_tracking_enabled())
 		return 0;
 
 	prev_ctx = this_cpu_read(context_tracking.state);
@@ -63,7 +64,8 @@ static inline enum ctx_state exception_enter(void)
 
 static inline void exception_exit(enum ctx_state prev_ctx)
 {
-	if (context_tracking_enabled()) {
+	if (!IS_ENABLED(CONFIG_HAVE_CONTEXT_TRACKING_OFFSTACK) &&
+	    context_tracking_enabled()) {
 		if (prev_ctx != CONTEXT_KERNEL)
 			context_tracking_enter(prev_ctx);
 	}
-- 
2.25.1

[PATCH 3/5] sched: Detect call to schedule from critical entry code

[Frederic Weisbecker]

Detect calls to schedule() between user_enter() and user_exit(). Those
are symptoms of early entry code that either forgot to protect a call
to schedule() inside exception_enter()/exception_exit() or, in the case
of HAVE_CONTEXT_TRACKING_OFFSTACK, enabled interrupts or preemption in
a wrong spot.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Phil Auld <pauld@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/sched/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2d95dc3f4644..d31a79e073e3 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4295,6 +4295,7 @@ static inline void schedule_debug(struct task_struct *prev, bool preempt)
 		preempt_count_set(PREEMPT_DISABLED);
 	}
 	rcu_sleep_check();
+	WARN_ON_ONCE(ct_state() == CONTEXT_USER);
 
 	profile_hit(SCHED_PROFILING, __builtin_return_address(0));
 
-- 
2.25.1

Re: [PATCH 3/5] sched: Detect call to schedule from critical entry code

[Peter Zijlstra]

On Mon, Oct 05, 2020 at 12:49:17PM +0200, Frederic Weisbecker wrote:
> Detect calls to schedule() between user_enter() and user_exit(). Those
> are symptoms of early entry code that either forgot to protect a call
> to schedule() inside exception_enter()/exception_exit() or, in the case
> of HAVE_CONTEXT_TRACKING_OFFSTACK, enabled interrupts or preemption in
> a wrong spot.
> 
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: Paul E. McKenney <paulmck@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Phil Auld <pauld@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
>  kernel/sched/core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 2d95dc3f4644..d31a79e073e3 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4295,6 +4295,7 @@ static inline void schedule_debug(struct task_struct *prev, bool preempt)
>  		preempt_count_set(PREEMPT_DISABLED);
>  	}
>  	rcu_sleep_check();
> +	WARN_ON_ONCE(ct_state() == CONTEXT_USER);

	SCHED_WARN_ON() ?

No point in unconditionally polluting that path. Although, per MeL, we
should probably invest in CONFIG_SCHED_DEBUG_I_MEANS_IT :/

Re: [PATCH 3/5] sched: Detect call to schedule from critical entry code

[Frederic Weisbecker]

On Mon, Oct 05, 2020 at 01:23:53PM +0200, Peter Zijlstra wrote:
> On Mon, Oct 05, 2020 at 12:49:17PM +0200, Frederic Weisbecker wrote:
> > Detect calls to schedule() between user_enter() and user_exit(). Those
> > are symptoms of early entry code that either forgot to protect a call
> > to schedule() inside exception_enter()/exception_exit() or, in the case
> > of HAVE_CONTEXT_TRACKING_OFFSTACK, enabled interrupts or preemption in
> > a wrong spot.
> > 
> > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > Cc: Marcelo Tosatti <mtosatti@redhat.com>
> > Cc: Paul E. McKenney <paulmck@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Phil Auld <pauld@redhat.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > ---
> >  kernel/sched/core.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 2d95dc3f4644..d31a79e073e3 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -4295,6 +4295,7 @@ static inline void schedule_debug(struct task_struct *prev, bool preempt)
> >  		preempt_count_set(PREEMPT_DISABLED);
> >  	}
> >  	rcu_sleep_check();
> > +	WARN_ON_ONCE(ct_state() == CONTEXT_USER);
> 
> 	SCHED_WARN_ON() ?

Bah! That's exactly what I was looking for.

> No point in unconditionally polluting that path. Although, per MeL, we
> should probably invest in CONFIG_SCHED_DEBUG_I_MEANS_IT :/

Because CONFIG_SCHED_DEBUG is often used by default on distros?

Thanks.

Re: [PATCH 3/5] sched: Detect call to schedule from critical entry code

[Mel Gorman]

On Mon, Oct 05, 2020 at 02:26:48PM +0200, Frederic Weisbecker wrote:
> On Mon, Oct 05, 2020 at 01:23:53PM +0200, Peter Zijlstra wrote:
> > On Mon, Oct 05, 2020 at 12:49:17PM +0200, Frederic Weisbecker wrote:
> > > Detect calls to schedule() between user_enter() and user_exit(). Those
> > > are symptoms of early entry code that either forgot to protect a call
> > > to schedule() inside exception_enter()/exception_exit() or, in the case
> > > of HAVE_CONTEXT_TRACKING_OFFSTACK, enabled interrupts or preemption in
> > > a wrong spot.
> > > 
> > > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > > Cc: Marcelo Tosatti <mtosatti@redhat.com>
> > > Cc: Paul E. McKenney <paulmck@kernel.org>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Phil Auld <pauld@redhat.com>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > ---
> > >  kernel/sched/core.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > > index 2d95dc3f4644..d31a79e073e3 100644
> > > --- a/kernel/sched/core.c
> > > +++ b/kernel/sched/core.c
> > > @@ -4295,6 +4295,7 @@ static inline void schedule_debug(struct task_struct *prev, bool preempt)
> > >  		preempt_count_set(PREEMPT_DISABLED);
> > >  	}
> > >  	rcu_sleep_check();
> > > +	WARN_ON_ONCE(ct_state() == CONTEXT_USER);
> > 
> > 	SCHED_WARN_ON() ?
> 
> Bah! That's exactly what I was looking for.
> 
> > No point in unconditionally polluting that path. Although, per MeL, we
> > should probably invest in CONFIG_SCHED_DEBUG_I_MEANS_IT :/
> 
> Because CONFIG_SCHED_DEBUG is often used by default on distros?
> 

SCHED_DEBUG is generally useful (e.g. figuring out weird topology problems
on new hardware). The overhead isn't too bad when schedstats are
disabled so it would be nice to avoid adding too much overhead via
SCHED_DEBUG.

Other debugging options -- not so much. A lot of them are useful for
development but there are people who request them be enabled anyway
thinking that they improve security somehow when in reality they might,
at best, detect a hardware issue that happens to hit a specific structure.

-- 
Mel Gorman
SUSE Labs

Re: [PATCH 3/5] sched: Detect call to schedule from critical entry code

[Frederic Weisbecker]

On Wed, Oct 07, 2020 at 10:34:36AM +0100, Mel Gorman wrote:
> On Mon, Oct 05, 2020 at 02:26:48PM +0200, Frederic Weisbecker wrote:
> > On Mon, Oct 05, 2020 at 01:23:53PM +0200, Peter Zijlstra wrote:
> > > On Mon, Oct 05, 2020 at 12:49:17PM +0200, Frederic Weisbecker wrote:
> > > > Detect calls to schedule() between user_enter() and user_exit(). Those
> > > > are symptoms of early entry code that either forgot to protect a call
> > > > to schedule() inside exception_enter()/exception_exit() or, in the case
> > > > of HAVE_CONTEXT_TRACKING_OFFSTACK, enabled interrupts or preemption in
> > > > a wrong spot.
> > > > 
> > > > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > > > Cc: Marcelo Tosatti <mtosatti@redhat.com>
> > > > Cc: Paul E. McKenney <paulmck@kernel.org>
> > > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > > Cc: Phil Auld <pauld@redhat.com>
> > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > > ---
> > > >  kernel/sched/core.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > > > index 2d95dc3f4644..d31a79e073e3 100644
> > > > --- a/kernel/sched/core.c
> > > > +++ b/kernel/sched/core.c
> > > > @@ -4295,6 +4295,7 @@ static inline void schedule_debug(struct task_struct *prev, bool preempt)
> > > >  		preempt_count_set(PREEMPT_DISABLED);
> > > >  	}
> > > >  	rcu_sleep_check();
> > > > +	WARN_ON_ONCE(ct_state() == CONTEXT_USER);
> > > 
> > > 	SCHED_WARN_ON() ?
> > 
> > Bah! That's exactly what I was looking for.
> > 
> > > No point in unconditionally polluting that path. Although, per MeL, we
> > > should probably invest in CONFIG_SCHED_DEBUG_I_MEANS_IT :/
> > 
> > Because CONFIG_SCHED_DEBUG is often used by default on distros?
> > 
> 
> SCHED_DEBUG is generally useful (e.g. figuring out weird topology problems
> on new hardware). The overhead isn't too bad when schedstats are
> disabled so it would be nice to avoid adding too much overhead via
> SCHED_DEBUG.
> 
> Other debugging options -- not so much. A lot of them are useful for
> development but there are people who request them be enabled anyway
> thinking that they improve security somehow when in reality they might,
> at best, detect a hardware issue that happens to hit a specific structure.


So we are good with SCHED_WARN_ON(), right?

I'll reissue with that.

Thanks.

[PATCH 4/5] context_tracking: Only define schedule_user() on !HAVE_CONTEXT_TRACKING_OFFSTACK archs

[Frederic Weisbecker]

schedule_user() was traditionally used by the entry code's tail to
preempt userspace after the call to user_enter(). Indeed the call to
user_enter() used to be performed upon syscall exit slow path which was
right before the last opportunity to schedule() while resuming to
userspace. The context tracking state had to be saved on the task stack
and set back to CONTEXT_KERNEL temporarily in order to safely switch to
another task.

Only a few archs use it now and those implementing
HAVE_CONTEXT_TRACKING_OFFSTACK definetly can't rely on it.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Phil Auld <pauld@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/sched/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d31a79e073e3..cb7afcfbb809 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4632,7 +4632,7 @@ void __sched schedule_idle(void)
 	} while (need_resched());
 }
 
-#ifdef CONFIG_CONTEXT_TRACKING
+#if defined(CONFIG_CONTEXT_TRACKING) && !defined(CONFIG_HAVE_CONTEXT_TRACKING_OFFSTACK)
 asmlinkage __visible void __sched schedule_user(void)
 {
 	/*
-- 
2.25.1

[PATCH 5/5] x86: Support HAVE_CONTEXT_TRACKING_OFFSTACK

[Frederic Weisbecker]

A lot of ground work has been performed on x86 entry code. Fragile path
between user_enter() and user_exit() have IRQs disabled. Uses of RCU and
intrumentation in these fragile areas have been explicitly annotated
and protected.

This architecture doesn't need exception_enter()/exception_exit()
anymore and has therefore earned CONFIG_HAVE_CONTEXT_TRACKING_OFFSTACK.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Phil Auld <pauld@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 7101ac64bb20..7a66865f32a8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -162,6 +162,7 @@ config X86
 	select HAVE_CMPXCHG_DOUBLE
 	select HAVE_CMPXCHG_LOCAL
 	select HAVE_CONTEXT_TRACKING		if X86_64
+	select HAVE_CONTEXT_TRACKING_OFFSTACK	if HAVE_CONTEXT_TRACKING
 	select HAVE_C_RECORDMCOUNT
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_DMA_CONTIGUOUS
-- 
2.25.1