On Tue, Sep 25, 2012 at 11:36:54AM -0700, Paul E. McKenney wrote:
> On Tue, Sep 25, 2012 at 08:28:23PM +0200, Sasha Levin wrote:
> > On 09/25/2012 02:06 PM, Frederic Weisbecker wrote:
> > > Sasha, sorry to burden you with more testing request.
> > > Could you please try out this new branch? It includes some fixes after Wu 
> > > Fenguang and
> > > Dan Carpenter reports (not related to your warnings though) and a patch 
> > > on the top
> > > of the pile to ensure I diagnosed well the problem, which return 
> > > immediately from
> > > rcu_user_*() APIs if we are in an interrupt.
> > > 
> > > This way we'll have a clearer view. I also would like to know if there 
> > > are other
> > > problems with the rcu user mode.
> > > 
> > > Thanks!
> > 
> > Alrighty, I don't see any warnings anymore.
> > 
> > I'll keep everything running just in case.
> 
> Very good news!!!  Thank you both!!!

So, I've pushed the fixes in a new branch.

Changes are:

* Added in_interrupt() checks in "rcu: New rcu_user_enter() and rcu_user_exit() 
APIs"
so that rcu_user_enter/exit() don't nest in rcu_irq_enter/exit().

We'll need a longer term solution I guess because of:
        - Irq bad nesting
        - An exception could happen in the middle of irq_exit(), although 
that's quite
          unlikely (ie: between sub_preempt_count(HARDIRQ) and 
add_preempt_count(SOFTIRQ) or between
          sub_preempt_count(SOFTIRQ or HARDIRQ) and rcu_irq_exit().
          May be I should rather check for (rdtp->dynticks_nesting & 
(DYNTICK_TASK_FLAG -1))
          to find out if we are in the middle of an irq from an RCU POV.


* Fix the !notifie_die(...) == NOTIFY_STOP to become notifie_die(...) != 
NOTIFY_STOP.
Reported by Wu Fenguang and Dan Carpenter. Fixes are folded inside:
"x86: Exception hooks for userspace RCU extended QS".

The branch (rebase on top of your rcu/idle) is:

git://github.com/fweisbec/linux-dynticks.git
        rcu/idle-for-v3.7-take5

Diff against your rcu/idle:

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index cb20776..3789675 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -232,8 +232,8 @@ DO_ERROR_INFO(X86_TRAP_AC, SIGBUS, "alignment check", 
alignment_check,
 dotraplinkage void do_stack_segment(struct pt_regs *regs, long error_code)
 {
        exception_enter(regs);
-       if (!notify_die(DIE_TRAP, "stack segment", regs, error_code,
-                      X86_TRAP_SS, SIGBUS) == NOTIFY_STOP) {
+       if (notify_die(DIE_TRAP, "stack segment", regs, error_code,
+                      X86_TRAP_SS, SIGBUS) != NOTIFY_STOP) {
                preempt_conditional_sti(regs);
                do_trap(X86_TRAP_SS, SIGBUS, "stack segment", regs, error_code, 
NULL);
                preempt_conditional_cli(regs);
@@ -285,8 +285,8 @@ do_general_protection(struct pt_regs *regs, long error_code)
 
                tsk->thread.error_code = error_code;
                tsk->thread.trap_nr = X86_TRAP_GP;
-               if (!notify_die(DIE_GPF, "general protection fault", regs, 
error_code,
-                              X86_TRAP_GP, SIGSEGV) == NOTIFY_STOP)
+               if (notify_die(DIE_GPF, "general protection fault", regs, 
error_code,
+                              X86_TRAP_GP, SIGSEGV) != NOTIFY_STOP)
                        die("general protection fault", regs, error_code);
                goto exit;
        }
@@ -678,8 +678,8 @@ dotraplinkage void do_iret_error(struct pt_regs *regs, long 
error_code)
        info.si_errno = 0;
        info.si_code = ILL_BADSTK;
        info.si_addr = NULL;
-       if (!notify_die(DIE_TRAP, "iret exception", regs, error_code,
-                       X86_TRAP_IRET, SIGILL) == NOTIFY_STOP) {
+       if (notify_die(DIE_TRAP, "iret exception", regs, error_code,
+                       X86_TRAP_IRET, SIGILL) != NOTIFY_STOP) {
                do_trap(X86_TRAP_IRET, SIGILL, "iret exception", regs, 
error_code,
                        &info);
        }
diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 72453cf..4fb2376 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -418,6 +418,17 @@ void rcu_user_enter(void)
        unsigned long flags;
        struct rcu_dynticks *rdtp;
 
+       /*
+        * Some contexts may involve an exception occuring in an irq,
+        * leading to that nesting:
+        * rcu_irq_enter() rcu_user_exit() rcu_user_exit() rcu_irq_exit()
+        * This would mess up the dyntick_nesting count though. And rcu_irq_*()
+        * helpers are enough to protect RCU uses inside the exception. So
+        * just return immediately if we detect we are in an IRQ.
+        */
+       if (in_interrupt())
+               return;
+
        WARN_ON_ONCE(!current->mm);
 
        local_irq_save(flags);
@@ -566,6 +577,17 @@ void rcu_user_exit(void)
        unsigned long flags;
        struct rcu_dynticks *rdtp;
 
+       /*
+        * Some contexts may involve an exception occuring in an irq,
+        * leading to that nesting:
+        * rcu_irq_enter() rcu_user_exit() rcu_user_exit() rcu_irq_exit()
+        * This would mess up the dyntick_nesting count though. And rcu_irq_*()
+        * helpers are enough to protect RCU uses inside the exception. So
+        * just return immediately if we detect we are in an IRQ.
+        */
+       if (in_interrupt())
+               return;
+
        local_irq_save(flags);
        rdtp = &__get_cpu_var(rcu_dynticks);
        if (rdtp->in_user) {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to