linux-kernel - Re: rcu_preempt self-detected stall on CPU from 4.5-rc3, since 3.17

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <683720290.37511.1459129458781.JavaMail.zimbra@efficios.com>
Date:	Mon, 28 Mar 2016 01:44:18 +0000 (UTC)
From:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:	Peter Zijlstra <peterz@...radead.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:	"Chatre, Reinette" <reinette.chatre@...el.com>,
	Jacob Pan <jacob.jun.pan@...ux.intel.com>,
	Josh Triplett <josh@...htriplett.org>,
	Ross Green <rgkernel@...il.com>,
	John Stultz <john.stultz@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	lkml <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Lai Jiangshan <jiangshanlai@...il.com>, dipankar@...ibm.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	rostedt <rostedt@...dmis.org>,
	David Howells <dhowells@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Darren Hart <dvhart@...ux.intel.com>,
	Frédéric Weisbecker <fweisbec@...il.com>,
	Oleg Nesterov <oleg@...hat.com>,
	pranith kumar <bobby.prani@...il.com>
Subject: Re: rcu_preempt self-detected stall on CPU from 4.5-rc3, since 3.17

----- On Mar 27, 2016, at 4:45 PM, Peter Zijlstra peterz@...radead.org wrote:

> On Sun, Mar 27, 2016 at 08:40:18AM -0700, Paul E. McKenney wrote:
>> Oh, and the patch I am running with is below.  I am running x86, and so
>> some other architectures would of course need the corresponding patch
>> on that architecture.
> 
>> -#define TIF_POLLING_NRFLAG	21	/* idle is polling for TIF_NEED_RESCHED */
>> +/* #define TIF_POLLING_NRFLAG	21	 idle is polling for TIF_NEED_RESCHED */
> 
> x86 is the only arch that really uses this heavily IIRC.
> 
> Most of the other archs need interrupts to wake up remote cores.
> 
> So what we try to do is avoid sending IPIs when the CPU is idle, for the
> remote wakeup case we use set_nr_if_polling() which sets
> TIF_NEED_RESCHED if TIF_POLLING_NRFLAG was set. If it wasn't, we'll send
> the IPI. Otherwise we rely on the idle loop to do sched_ttwu_pending()
> when it breaks out of loop due to TIF_NEED_RESCHED.
> 
> But, you need hotplug for this to happen, right?

My understanding is that this seems to be detection of failures to be
awakened for a long time on idle CPUs. It therefore seems to be more
idle-related than cpu hotplug-related. I'm not saying that there is
no issue with hotplug, just that the investigation so far seems to
target mostly idle systems, AFAIK without stressing hotplug.

> 
> We should not be migrating towards, or waking on, CPUs no longer present
> in cpu_active_map, and there is a rcu/sched_sync() after clearing that
> bit. Furthermore, migration_call() does a sched_ttwu_pending() (waking
> any remaining stragglers) before we migrate all runnable tasks off the
> dying CPU.
> 
> 
> 
> The other interesting case would be resched_cpu(), which uses
> set_nr_and_not_polling() to kick a remote cpu to call schedule(). It
> atomically sets TIF_NEED_RESCHED and returns if TIF_POLLING_NRFLAG was
> not set. If indeed not, it will send an IPI.
> 
> This assumes the idle 'exit' path will do the same as the IPI does; and
> if you look at cpu_idle_loop() it does indeed do both
> preempt_fold_need_resched() and sched_ttwu_pending().
> 
> Note that one cannot rely on irq_enter()/irq_exit() being called for the
> scheduler IPI.

Looking at commit e3baac47f0e82c4be632f4f97215bb93bf16b342 :

set_nr_if_polling() returns true if the ti->flags read has the
_TIF_NEED_RESCHED bit set, which will skip the IPI.

But it seems weird. The side that calls set_nr_if_polling()
does the following:
1) llist_add(&p->wake_entry, &cpu_rq(cpu)->wake_list)
2) set_nr_if_polling(rq->idle)
3) (don't do smp_send_reschedule(cpu) since set_nr_if_polling() returned
   true)

The idle loop does:
1) __current_set_polling()
2) __current_clr_polling()
3) smp_mb__after_atomic()
4) sched_ttwu_pending()
5) schedule_preempt_disabled()
   -> This will clear the TIF_NEED_RESCHED flag

While the idle loop is in sched_ttwu_pending(), after
it has done the llist_del_all() (thus has grabbed all the
list entries), TIF_NEED_RESCHED is still set. If both list_all and
set_nr_if_polling() are called right after the llist_del_all(), we
will end up in a situation where we have an entry in the list, but
there won't be any reschedule sent on the idle CPU until something
else awakens it. On a _very_ idle CPU, this could take some time.

set_nr_and_not_polling() don't seem to have the same issue, because
it does not return true if TIF_NEED_RESCHED is observed as being
already set: it really just depends on the state of the TIF_POLLING_NRFLAG
bit.

Am I missing something important ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com