Re: [PATCH -rt 1/9] preempt rcu: check for underflow

2007-07-30 Thread Daniel Walker
On Mon, 2007-07-30 at 11:22 +0200, Ingo Molnar wrote:
> * Daniel Walker <[EMAIL PROTECTED]> wrote:
> 
> > +   /*
> > +* If our rcu_read_lock_nesting went negative, likely
> > +* something is wrong..
> > +*/
> > +   WARN_ON(current->rcu_read_lock_nesting < 0);
> 
> have you actually caught any rcu locking problem this way? Double 
> unlocks should be caught by lockdep already, at a higher level.
> 
> in any case i've added a slightly different form of this change to the 
> -rt queue that will also check for counter overflows. But i'm not sure 
> we want to litter the code with trivial checks like this, so i'm keeping 
> it separate and if it does not trigger anything real i'll remove it.

I haven't caught anything with it, but this code would have made it much
easier to catch the single rcu unlock in sys_sched_yield() which was
silent in PREEMPT_RT, and hung !PREEMPT_RT ..

It's fine with me, if you have another method.

Daniel

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


Re: [PATCH -rt 1/9] preempt rcu: check for underflow

2007-07-30 Thread Ingo Molnar

* Daniel Walker <[EMAIL PROTECTED]> wrote:

> + /*
> +  * If our rcu_read_lock_nesting went negative, likely
> +  * something is wrong..
> +  */
> + WARN_ON(current->rcu_read_lock_nesting < 0);

have you actually caught any rcu locking problem this way? Double 
unlocks should be caught by lockdep already, at a higher level.

in any case i've added a slightly different form of this change to the 
-rt queue that will also check for counter overflows. But i'm not sure 
we want to litter the code with trivial checks like this, so i'm keeping 
it separate and if it does not trigger anything real i'll remove it.

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


Re: [PATCH -rt 1/9] preempt rcu: check for underflow

2007-07-30 Thread Ingo Molnar

* Daniel Walker [EMAIL PROTECTED] wrote:

 + /*
 +  * If our rcu_read_lock_nesting went negative, likely
 +  * something is wrong..
 +  */
 + WARN_ON(current-rcu_read_lock_nesting  0);

have you actually caught any rcu locking problem this way? Double 
unlocks should be caught by lockdep already, at a higher level.

in any case i've added a slightly different form of this change to the 
-rt queue that will also check for counter overflows. But i'm not sure 
we want to litter the code with trivial checks like this, so i'm keeping 
it separate and if it does not trigger anything real i'll remove it.

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


Re: [PATCH -rt 1/9] preempt rcu: check for underflow

2007-07-30 Thread Daniel Walker
On Mon, 2007-07-30 at 11:22 +0200, Ingo Molnar wrote:
 * Daniel Walker [EMAIL PROTECTED] wrote:
 
  +   /*
  +* If our rcu_read_lock_nesting went negative, likely
  +* something is wrong..
  +*/
  +   WARN_ON(current-rcu_read_lock_nesting  0);
 
 have you actually caught any rcu locking problem this way? Double 
 unlocks should be caught by lockdep already, at a higher level.
 
 in any case i've added a slightly different form of this change to the 
 -rt queue that will also check for counter overflows. But i'm not sure 
 we want to litter the code with trivial checks like this, so i'm keeping 
 it separate and if it does not trigger anything real i'll remove it.

I haven't caught anything with it, but this code would have made it much
easier to catch the single rcu unlock in sys_sched_yield() which was
silent in PREEMPT_RT, and hung !PREEMPT_RT ..

It's fine with me, if you have another method.

Daniel

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


Re: [PATCH -rt 1/9] preempt rcu: check for underflow

2007-07-29 Thread Paul E. McKenney
On Sun, Jul 29, 2007 at 07:45:35PM -0700, Daniel Walker wrote:
> Simple WARN_ON to catch any underflow in rcu_read_lock_nesting.
> 
> Signed-off-by: Daniel Walker <[EMAIL PROTECTED]>

Acked-by: Paul E. McKenney <[EMAIL PROTECTED]>

> ---
>  kernel/rcupreempt.c |6 ++
>  1 file changed, 6 insertions(+)
> 
> Index: linux-2.6.22/kernel/rcupreempt.c
> ===
> --- linux-2.6.22.orig/kernel/rcupreempt.c
> +++ linux-2.6.22/kernel/rcupreempt.c
> @@ -157,6 +157,12 @@ void __rcu_read_unlock(void)
>   }
> 
>   local_irq_restore(oldirq);
> +
> + /*
> +  * If our rcu_read_lock_nesting went negative, likely
> +  * something is wrong..
> +  */
> + WARN_ON(current->rcu_read_lock_nesting < 0);
>  }
> 
>  static void __rcu_advance_callbacks(void)
> 
> -- 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH -rt 1/9] preempt rcu: check for underflow

2007-07-29 Thread Daniel Walker
Simple WARN_ON to catch any underflow in rcu_read_lock_nesting.

Signed-off-by: Daniel Walker <[EMAIL PROTECTED]>

---
 kernel/rcupreempt.c |6 ++
 1 file changed, 6 insertions(+)

Index: linux-2.6.22/kernel/rcupreempt.c
===
--- linux-2.6.22.orig/kernel/rcupreempt.c
+++ linux-2.6.22/kernel/rcupreempt.c
@@ -157,6 +157,12 @@ void __rcu_read_unlock(void)
}
 
local_irq_restore(oldirq);
+
+   /*
+* If our rcu_read_lock_nesting went negative, likely
+* something is wrong..
+*/
+   WARN_ON(current->rcu_read_lock_nesting < 0);
 }
 
 static void __rcu_advance_callbacks(void)

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


[PATCH -rt 1/9] preempt rcu: check for underflow

2007-07-29 Thread Daniel Walker
Simple WARN_ON to catch any underflow in rcu_read_lock_nesting.

Signed-off-by: Daniel Walker [EMAIL PROTECTED]

---
 kernel/rcupreempt.c |6 ++
 1 file changed, 6 insertions(+)

Index: linux-2.6.22/kernel/rcupreempt.c
===
--- linux-2.6.22.orig/kernel/rcupreempt.c
+++ linux-2.6.22/kernel/rcupreempt.c
@@ -157,6 +157,12 @@ void __rcu_read_unlock(void)
}
 
local_irq_restore(oldirq);
+
+   /*
+* If our rcu_read_lock_nesting went negative, likely
+* something is wrong..
+*/
+   WARN_ON(current-rcu_read_lock_nesting  0);
 }
 
 static void __rcu_advance_callbacks(void)

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


Re: [PATCH -rt 1/9] preempt rcu: check for underflow

2007-07-29 Thread Paul E. McKenney
On Sun, Jul 29, 2007 at 07:45:35PM -0700, Daniel Walker wrote:
 Simple WARN_ON to catch any underflow in rcu_read_lock_nesting.
 
 Signed-off-by: Daniel Walker [EMAIL PROTECTED]

Acked-by: Paul E. McKenney [EMAIL PROTECTED]

 ---
  kernel/rcupreempt.c |6 ++
  1 file changed, 6 insertions(+)
 
 Index: linux-2.6.22/kernel/rcupreempt.c
 ===
 --- linux-2.6.22.orig/kernel/rcupreempt.c
 +++ linux-2.6.22/kernel/rcupreempt.c
 @@ -157,6 +157,12 @@ void __rcu_read_unlock(void)
   }
 
   local_irq_restore(oldirq);
 +
 + /*
 +  * If our rcu_read_lock_nesting went negative, likely
 +  * something is wrong..
 +  */
 + WARN_ON(current-rcu_read_lock_nesting  0);
  }
 
  static void __rcu_advance_callbacks(void)
 
 -- 
 -
 To unsubscribe from this list: send the line unsubscribe linux-rt-users in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/