On Thu, 09 Nov 2017 15:17:28 +0000 Ben Hutchings ben.hutchings@codethink.co.uk wrote:
On Mon, 2017-10-16 at 18:11 +0200, gregkh@linuxfoundation.org wrote:
4.4-stable review patch. If anyone has any objections, please let me know.
From: Paul E. McKenney paulmck@linux.vnet.ibm.com
commit 28585a832602747cbfa88ad8934013177a3aae38 upstream.
A number of architecture invoke rcu_irq_enter() on exception entry in order to allow RCU read-side critical sections in the exception handler when the exception is from an idle or nohz_full CPU. This works, at least unless the exception happens in an NMI handler. In that case, rcu_nmi_enter() would already have exited the extended quiescent state, which would mean that rcu_irq_enter() would (incorrectly) cause RCU to think that it is again in an extended quiescent state. This will in turn result in lockdep splats in response to later RCU read-side critical sections.
This commit therefore causes rcu_irq_enter() and rcu_irq_exit() to take no action if there is an rcu_nmi_enter() in effect, thus avoiding the unscheduled return to RCU quiescent state. This in turn should make the kernel safe for on-demand RCU voyeurism.
Link: http://lkml.kernel.org/r/20170922211022.GA18084@linux.vnet.ibm.com
Cc: stable@vger.kernel.org Fixes: 0be964be0 ("module: Sanitize RCU usage and locking")
Reported-by: Steven Rostedt rostedt@goodmis.org Signed-off-by: Paul E. McKenney paulmck@linux.vnet.ibm.com Signed-off-by: Steven Rostedt (VMware) rostedt@goodmis.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
kernel/rcu/tree.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
--- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -759,6 +759,12 @@ void rcu_irq_exit(void) local_irq_save(flags); rdtp = this_cpu_ptr(&rcu_dynticks);
- /* Page faults can happen in NMI handlers, so check... */
- if (READ_ONCE(rdtp->dynticks_nmi_nesting))
return;Shouldn't there be a local_irq_restore() on this return path? Or does this condition imply that IRQs were already disabled?
It does, but there still should be a local_irq_restore(), because it will might confuse lockdep (lockdep keeps track of these), and it just looks bad.
- RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_exit() invoked with irqs enabled!!!");
I don't see why you added RCU_LOCKDEP_WARN() here. Prior to 4.5 it's not an error to call this function with IRQs disabled. And after calling local_irq_save(), it's redundant to assert that IRQs are disabled.
good point.
oldval = rdtp->dynticks_nesting; rdtp->dynticks_nesting--; WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && @@ -887,6 +893,12 @@ void rcu_irq_enter(void) local_irq_save(flags); rdtp = this_cpu_ptr(&rcu_dynticks);
- /* Page faults can happen in NMI handlers, so check... */
- if (READ_ONCE(rdtp->dynticks_nmi_nesting))
return;- RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_enter() invoked with irqs enabled!!!");
Same problems here.
-- Steve