[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.20.1705160923070.1934@nanos>
Date: Tue, 16 May 2017 09:33:52 +0200 (CEST)
From: Thomas Gleixner <tglx@...utronix.de>
To: Peter Zijlstra <peterz@...radead.org>
cc: Steven Rostedt <rostedt@...dmis.org>,
LKML <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>,
Mark Rutland <mark.rutland@....com>
Subject: Re: [patch 17/18] sched: Enable might_sleep() checks early
On Tue, 16 May 2017, Peter Zijlstra wrote:
> On Mon, May 15, 2017 at 09:12:03PM +0200, Thomas Gleixner wrote:
> > On Mon, 15 May 2017, Steven Rostedt wrote:
> >
> > > On Sun, 14 May 2017 20:27:33 +0200
> > > Thomas Gleixner <tglx@...utronix.de> wrote:
> > >
> > > > might_sleep() 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.
> > >
> > > Looking at commit b433c3d4549ae749, it appears that on very slow
> > > machines, there is a possibility that the init task can start running.
> > > Should system_state be updated before that complete() is called?
> >
> > That commit is magic voodoo with exactly no effect at all.
> >
> > rest_init() is called with preemption disabled and nothing can schedule
> > there _before_ schedule_preempt_disabled().
> >
> > Both threads - init task and kthreadd - are only created and woken up. They
> > cannot get on the CPU simply because preemption is disabled. And this was
> > the case back then in 2.6.35 as well.
> >
> > It does not matter at all whether the machine is slow or not. That
> > completion is pointless.
> >
> > Peter, can you explain what the heck this patch is actually doing?
>
> Argh.. what a shit Changelog, who wrote that crap!?
Indeed.
> So the problem was with PREEMPT_VOLUNTARY (where, as you know,
> preempt_disable() has no meaning).
>
> Supposedly there's a might_sleep()/cond_resched() point somewhere around
> there (every alloc in the fork path for example), which will happily
> reschedule us.
Darn, forgot about PREEMPT_VOLUNTARY and that excellent changelog does not
mention it either.
> So if we schedule to the kernel_init() task before we set kthreadd_task
> we'll try and spawn kthreads and OOPS.
So back to Stevens question. No, we can't set the state earlier than right
before schedule() simply because with PREEMPT preemption _is_ actually
disabled and kernel_kthread() will trigger might_sleep() splats.
What a mess.
Thanks,
tglx
Powered by blists - more mailing lists