Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 5 Jan 2021 13:57:22 +0100
From:   Frederic Weisbecker <frederic@kernel.org>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
        Ingo Molnar <mingo@kernel.org>,
        Fabio Estevam <festevam@gmail.com>, stable@vger.kernel.org,
        Thomas Gleixner <tglx@linutronix.de>,
        "Paul E . McKenney" <paulmck@kernel.org>,
        Len Brown <lenb@kernel.org>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        NXP Linux Team <linux-imx@nxp.com>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>
Subject: Re: [PATCH 1/4] sched/idle: Fix missing need_resched() check after
 rcu_idle_enter()
Message-ID: <20210105125722.GA68490@lothringen>
References: <20210104152058.36642-1-frederic@kernel.org>
 <20210104152058.36642-2-frederic@kernel.org>
 <20210105095503.GF3040@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210105095503.GF3040@hirez.programming.kicks-ass.net>
Precedence: bulk

On Tue, Jan 05, 2021 at 10:55:03AM +0100, Peter Zijlstra wrote:
> On Mon, Jan 04, 2021 at 04:20:55PM +0100, Frederic Weisbecker wrote:
> > Entering RCU idle mode may cause a deferred wake up of an RCU NOCB_GP
> > kthread (rcuog) to be serviced.
> > 
> > Usually a wake up happening while running the idle task is spotted in
> > one of the need_resched() checks carefully placed within the idle loop
> > that can break to the scheduler.
> 
> Urgh, this is horrific and fragile :/ You having had to audit and fix a
> number of rcu_idle_enter() callers should've made you realize that
> making rcu_idle_enter() return something would've been saner.
> 
> Also, I might hope that when RCU does do that wakeup, it will not have
> put RCU in idle mode? So it is a natural 'fail' state for
> rcu_idle_enter(), *sigh* it continues to put RCU to sleep, so that needs
> fixing too.

Heh, yes you're right, that looks saner.

> 
> I'm thinking that rcu_user_enter() will have the exact same problem? Did
> you audit that?

Yes and I wanted to fix it seperately since it's a bit harder to fix because
we are past the last need_resched() check, all syscall exit works, lockdep
hardirqs on entry prep, tracing hardirqs on, etc... I need to manage to
rollback safely and cleanly.

Unless I can decouple the wakeup from rcu_user_enter() and put it around the
exit_to_user_mode_loop(). But then I must make sure that call_rcu() isn't called
afterward.

> 
> Something like the below, combined with a fixup for all callers (which
> the compiler will help us find thanks to __must_check).

Right, I just need to make sure that the wake up is local as the kthread
awaken can be queued anywhere. But a simple need_resched() check after the
wake up should be fine to get that.

Thanks.