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@xxxxxxxxxx>
> > > Cc: Marcelo Tosatti <mtosatti@xxxxxxxxxx>
> > > Cc: Paul E. McKenney <paulmck@xxxxxxxxxx>
> > > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > > Cc: Phil Auld <pauld@xxxxxxxxxx>
> > > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > > ---
> > >  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