Re: [PATCH 2/4] kernel.h: Add non_block_start/end()

Peter Zijlstra
2018-12-10T16:30:09Z

On Mon, Dec 10, 2018 at 05:20:10PM +0100, Michal Hocko wrote:
> > OK, no real objections to the thing. Just so long we're all on the same
> > page as to what it does and doesn't do ;-)
>
> I am not really sure whether there are other potential users besides
> this one and whether the check as such is justified.

It's a debug option...

> > I suppose you could extend the check to include schedule_debug() as
> > well, maybe something like:
>
> Do you mean to make the check cheaper?

Nah, so the patch only touched might_sleep(), the below touches
schedule().

If there were a patch that hits schedule() without going through a
might_sleep() (rare in practise I think, but entirely possible) then you
won't get a splat without something like the below on top.

> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index f66920173370..b1aaa278f1af 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -3278,13 +3278,18 @@ static noinline void __schedule_bug(struct task_struct *prev)
> > /*
> > * Various schedule()-time debugging checks and statistics:
> > */
> > -static inline void schedule_debug(struct task_struct *prev)
> > +static inline void schedule_debug(struct task_struct *prev, bool preempt)
> > {
> > #ifdef CONFIG_SCHED_STACK_END_CHECK
> > if (task_stack_end_corrupted(prev))
> > panic("corrupted stack end detected inside scheduler\n");
> > #endif
> >
> > +#ifdef CONFIG_DEBUG_ATOMIC_SLEEP
> > + if (!preempt && prev->state && prev->non_block_count)
> > + // splat
> > +#endif
> > +
> > if (unlikely(in_atomic_preempt_off())) {
> > __schedule_bug(prev);
> > preempt_count_set(PREEMPT_DISABLED);
> > @@ -3391,7 +3396,7 @@ static void __sched notrace __schedule(bool preempt)
> > rq = cpu_rq(cpu);
> > prev = rq->curr;
> >
> > - schedule_debug(prev);
> > + schedule_debug(prev, preempt);
> >
> > if (sched_feat(HRTICK))
> > hrtick_clear(rq);
>
> --
> Michal Hocko
> SUSE Labs