Re: [RFC][PATCH 1/6] lockdep: annotate rcu_read_{,un}lock{,_bh}

2007-09-19 Thread Paul E. McKenney 
On Wed, Sep 19, 2007 at 12:41:26PM +0200, Peter Zijlstra wrote:
> lockdep annotate rcu_read_{,un}lock{,_bh} in order to catch imbalanced
> usage.

In my message yesterday, I forgot about srcu_read_lock() and
srcu_read_unlock().  :-/ Here is a proto-patch, untested,
probably does not compile.

One interesting side-effect of the overall patch is that one must select
CONFIG_PREEMPT in order for a CONFIG_DEBUG_LOCK_ALLOC build to compile.
Might be OK, but thought I should mention it.

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

diff -urpNa -X dontdiff linux-2.6.22-rcudep/kernel/srcu.c 
linux-2.6.22-srcudep/kernel/srcu.c
--- linux-2.6.22-rcudep/kernel/srcu.c   2007-07-08 16:32:17.0 -0700
+++ linux-2.6.22-srcudep/kernel/srcu.c  2007-09-19 12:50:00.0 -0700
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * init_srcu_struct - initialize a sleep-RCU structure
@@ -116,6 +117,7 @@ int srcu_read_lock(struct srcu_struct *s
per_cpu_ptr(sp->per_cpu_ref, smp_processor_id())->c[idx]++;
srcu_barrier();  /* ensure compiler won't misorder critical section. */
preempt_enable();
+   rcu_read_acquire();
return idx;
 }
 
@@ -131,6 +133,7 @@ int srcu_read_lock(struct srcu_struct *s
  */
 void srcu_read_unlock(struct srcu_struct *sp, int idx)
 {
+   rcu_read_release();
preempt_disable();
srcu_barrier();  /* ensure compiler won't misorder critical section. */
per_cpu_ptr(sp->per_cpu_ref, smp_processor_id())->c[idx]--;

> Signed-off-by: Peter Zijlstra <[EMAIL PROTECTED]>
> ---
>  include/linux/lockdep.h  |7 +++
>  include/linux/rcupdate.h |   12 
>  kernel/rcupdate.c|8 
>  3 files changed, 27 insertions(+)
> 
> Index: linux-2.6/include/linux/lockdep.h
> ===
> --- linux-2.6.orig/include/linux/lockdep.h
> +++ linux-2.6/include/linux/lockdep.h
> @@ -252,6 +252,13 @@ extern void lockdep_init_map(struct lock
>struct lock_class_key *key, int subclass);
> 
>  /*
> + * To initialize a lockdep_map statically use this macro.
> + * Note that _name must not be NULL.
> + */
> +#define STATIC_LOCKDEP_MAP_INIT(_name, _key) \
> + { .name = (_name), .key = (void *)(_key), }
> +
> +/*
>   * Reinitialize a lock key - for cases where there is special locking or
>   * special initialization of locks so that the validator gets the scope
>   * of dependencies wrong: they are either too broad (they need a class-split)
> Index: linux-2.6/include/linux/rcupdate.h
> ===
> --- linux-2.6.orig/include/linux/rcupdate.h
> +++ linux-2.6/include/linux/rcupdate.h
> @@ -41,6 +41,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  /**
>   * struct rcu_head - callback structure for use with RCU
> @@ -133,6 +134,15 @@ static inline void rcu_bh_qsctr_inc(int 
>  extern int rcu_pending(int cpu);
>  extern int rcu_needs_cpu(int cpu);
> 
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +extern struct lockdep_map rcu_lock_map;
> +# define rcu_read_acquire()  lock_acquire(_lock_map, 0, 0, 2, 1, 
> _THIS_IP_)
> +# define rcu_read_release()  lock_release(_lock_map, 1, _THIS_IP_)
> +#else
> +# define rcu_read_acquire()  do { } while (0)
> +# define rcu_read_release()  do { } while (0)
> +#endif
> +
>  /**
>   * rcu_read_lock - mark the beginning of an RCU read-side critical section.
>   *
> @@ -166,6 +176,7 @@ extern int rcu_needs_cpu(int cpu);
>   do { \
>   preempt_disable(); \
>   __acquire(RCU); \
> + rcu_read_acquire(); \
>   } while(0)
> 
>  /**
> @@ -175,6 +186,7 @@ extern int rcu_needs_cpu(int cpu);
>   */
>  #define rcu_read_unlock() \
>   do { \
> + rcu_read_release(); \
>   __release(RCU); \
>   preempt_enable(); \
>   } while(0)
> @@ -216,6 +218,7 @@ extern struct lockdep_map rcu_lock_map;
>   do { \
>   local_bh_disable(); \
>   __acquire(RCU_BH); \
> + rcu_read_acquire(); \
>   } while(0)
> 
>  /*
> @@ -225,6 +228,7 @@ extern struct lockdep_map rcu_lock_map;
>   */
>  #define rcu_read_unlock_bh() \
>   do { \
> + rcu_read_release(); \
>   __release(RCU_BH); \
>   local_bh_enable(); \
>   } while(0)
> Index: linux-2.6/kernel/rcupdate.c
> ===
> --- linux-2.6.orig/kernel/rcupdate.c
> +++ linux-2.6/kernel/rcupdate.c
> @@ -48,6 +48,14 @@
>  #include 
>  #include 
> 
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +static struct lock_class_key rcu_lock_key;
> +struct lockdep_map rcu_lock_map =
> + STATIC_LOCKDEP_MAP_INIT("rcu_read_lock", _lock_key);
> +
> +EXPORT_SYMBOL_GPL(rcu_lock_map);
> +#endif
> +
>  /* Definition for rcupdate control block. */
>  static struct rcu_ctrlblk rcu_ctrlblk = {
>   .cur = -300,
> 
> --
> 
-
To unsubscribe

[RFC][PATCH 1/6] lockdep: annotate rcu_read_{,un}lock{,_bh}

2007-09-19 Thread Peter Zijlstra 
lockdep annotate rcu_read_{,un}lock{,_bh} in order to catch imbalanced
usage.

Signed-off-by: Peter Zijlstra <[EMAIL PROTECTED]>
---
 include/linux/lockdep.h  |7 +++
 include/linux/rcupdate.h |   12 
 kernel/rcupdate.c|8 
 3 files changed, 27 insertions(+)

Index: linux-2.6/include/linux/lockdep.h
===
--- linux-2.6.orig/include/linux/lockdep.h
+++ linux-2.6/include/linux/lockdep.h
@@ -252,6 +252,13 @@ extern void lockdep_init_map(struct lock
 struct lock_class_key *key, int subclass);
 
 /*
+ * To initialize a lockdep_map statically use this macro.
+ * Note that _name must not be NULL.
+ */
+#define STATIC_LOCKDEP_MAP_INIT(_name, _key) \
+   { .name = (_name), .key = (void *)(_key), }
+
+/*
  * Reinitialize a lock key - for cases where there is special locking or
  * special initialization of locks so that the validator gets the scope
  * of dependencies wrong: they are either too broad (they need a class-split)
Index: linux-2.6/include/linux/rcupdate.h
===
--- linux-2.6.orig/include/linux/rcupdate.h
+++ linux-2.6/include/linux/rcupdate.h
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * struct rcu_head - callback structure for use with RCU
@@ -133,6 +134,15 @@ static inline void rcu_bh_qsctr_inc(int 
 extern int rcu_pending(int cpu);
 extern int rcu_needs_cpu(int cpu);
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+extern struct lockdep_map rcu_lock_map;
+# define rcu_read_acquire()lock_acquire(_lock_map, 0, 0, 2, 1, 
_THIS_IP_)
+# define rcu_read_release()lock_release(_lock_map, 1, _THIS_IP_)
+#else
+# define rcu_read_acquire()do { } while (0)
+# define rcu_read_release()do { } while (0)
+#endif
+
 /**
  * rcu_read_lock - mark the beginning of an RCU read-side critical section.
  *
@@ -166,6 +176,7 @@ extern int rcu_needs_cpu(int cpu);
do { \
preempt_disable(); \
__acquire(RCU); \
+   rcu_read_acquire(); \
} while(0)
 
 /**
@@ -175,6 +186,7 @@ extern int rcu_needs_cpu(int cpu);
  */
 #define rcu_read_unlock() \
do { \
+   rcu_read_release(); \
__release(RCU); \
preempt_enable(); \
} while(0)
@@ -216,6 +218,7 @@ extern struct lockdep_map rcu_lock_map;
do { \
local_bh_disable(); \
__acquire(RCU_BH); \
+   rcu_read_acquire(); \
} while(0)
 
 /*
@@ -225,6 +228,7 @@ extern struct lockdep_map rcu_lock_map;
  */
 #define rcu_read_unlock_bh() \
do { \
+   rcu_read_release(); \
__release(RCU_BH); \
local_bh_enable(); \
} while(0)
Index: linux-2.6/kernel/rcupdate.c
===
--- linux-2.6.orig/kernel/rcupdate.c
+++ linux-2.6/kernel/rcupdate.c
@@ -48,6 +48,14 @@
 #include 
 #include 
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+static struct lock_class_key rcu_lock_key;
+struct lockdep_map rcu_lock_map =
+   STATIC_LOCKDEP_MAP_INIT("rcu_read_lock", _lock_key);
+
+EXPORT_SYMBOL_GPL(rcu_lock_map);
+#endif
+
 /* Definition for rcupdate control block. */
 static struct rcu_ctrlblk rcu_ctrlblk = {
.cur = -300,

--

-
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/

[RFC][PATCH 1/6] lockdep: annotate rcu_read_{,un}lock{,_bh}

2007-09-19 Thread Peter Zijlstra 
lockdep annotate rcu_read_{,un}lock{,_bh} in order to catch imbalanced
usage.

Signed-off-by: Peter Zijlstra [EMAIL PROTECTED]
---
 include/linux/lockdep.h  |7 +++
 include/linux/rcupdate.h |   12 
 kernel/rcupdate.c|8 
 3 files changed, 27 insertions(+)

Index: linux-2.6/include/linux/lockdep.h
===
--- linux-2.6.orig/include/linux/lockdep.h
+++ linux-2.6/include/linux/lockdep.h
@@ -252,6 +252,13 @@ extern void lockdep_init_map(struct lock
 struct lock_class_key *key, int subclass);
 
 /*
+ * To initialize a lockdep_map statically use this macro.
+ * Note that _name must not be NULL.
+ */
+#define STATIC_LOCKDEP_MAP_INIT(_name, _key) \
+   { .name = (_name), .key = (void *)(_key), }
+
+/*
  * Reinitialize a lock key - for cases where there is special locking or
  * special initialization of locks so that the validator gets the scope
  * of dependencies wrong: they are either too broad (they need a class-split)
Index: linux-2.6/include/linux/rcupdate.h
===
--- linux-2.6.orig/include/linux/rcupdate.h
+++ linux-2.6/include/linux/rcupdate.h
@@ -41,6 +41,7 @@
 #include linux/percpu.h
 #include linux/cpumask.h
 #include linux/seqlock.h
+#include linux/lockdep.h
 
 /**
  * struct rcu_head - callback structure for use with RCU
@@ -133,6 +134,15 @@ static inline void rcu_bh_qsctr_inc(int 
 extern int rcu_pending(int cpu);
 extern int rcu_needs_cpu(int cpu);
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+extern struct lockdep_map rcu_lock_map;
+# define rcu_read_acquire()lock_acquire(rcu_lock_map, 0, 0, 2, 1, 
_THIS_IP_)
+# define rcu_read_release()lock_release(rcu_lock_map, 1, _THIS_IP_)
+#else
+# define rcu_read_acquire()do { } while (0)
+# define rcu_read_release()do { } while (0)
+#endif
+
 /**
  * rcu_read_lock - mark the beginning of an RCU read-side critical section.
  *
@@ -166,6 +176,7 @@ extern int rcu_needs_cpu(int cpu);
do { \
preempt_disable(); \
__acquire(RCU); \
+   rcu_read_acquire(); \
} while(0)
 
 /**
@@ -175,6 +186,7 @@ extern int rcu_needs_cpu(int cpu);
  */
 #define rcu_read_unlock() \
do { \
+   rcu_read_release(); \
__release(RCU); \
preempt_enable(); \
} while(0)
@@ -216,6 +218,7 @@ extern struct lockdep_map rcu_lock_map;
do { \
local_bh_disable(); \
__acquire(RCU_BH); \
+   rcu_read_acquire(); \
} while(0)
 
 /*
@@ -225,6 +228,7 @@ extern struct lockdep_map rcu_lock_map;
  */
 #define rcu_read_unlock_bh() \
do { \
+   rcu_read_release(); \
__release(RCU_BH); \
local_bh_enable(); \
} while(0)
Index: linux-2.6/kernel/rcupdate.c
===
--- linux-2.6.orig/kernel/rcupdate.c
+++ linux-2.6/kernel/rcupdate.c
@@ -48,6 +48,14 @@
 #include linux/cpu.h
 #include linux/mutex.h
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+static struct lock_class_key rcu_lock_key;
+struct lockdep_map rcu_lock_map =
+   STATIC_LOCKDEP_MAP_INIT(rcu_read_lock, rcu_lock_key);
+
+EXPORT_SYMBOL_GPL(rcu_lock_map);
+#endif
+
 /* Definition for rcupdate control block. */
 static struct rcu_ctrlblk rcu_ctrlblk = {
.cur = -300,

--

-
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: [RFC][PATCH 1/6] lockdep: annotate rcu_read_{,un}lock{,_bh}

2007-09-19 Thread Paul E. McKenney 
On Wed, Sep 19, 2007 at 12:41:26PM +0200, Peter Zijlstra wrote:
 lockdep annotate rcu_read_{,un}lock{,_bh} in order to catch imbalanced
 usage.

In my message yesterday, I forgot about srcu_read_lock() and
srcu_read_unlock().  :-/ Here is a proto-patch, untested,
probably does not compile.

One interesting side-effect of the overall patch is that one must select
CONFIG_PREEMPT in order for a CONFIG_DEBUG_LOCK_ALLOC build to compile.
Might be OK, but thought I should mention it.

Signed-off-by: Paul E. McKenney [EMAIL PROTECTED]

diff -urpNa -X dontdiff linux-2.6.22-rcudep/kernel/srcu.c 
linux-2.6.22-srcudep/kernel/srcu.c
--- linux-2.6.22-rcudep/kernel/srcu.c   2007-07-08 16:32:17.0 -0700
+++ linux-2.6.22-srcudep/kernel/srcu.c  2007-09-19 12:50:00.0 -0700
@@ -33,6 +33,7 @@
 #include linux/slab.h
 #include linux/smp.h
 #include linux/srcu.h
+#include linux/rcupdate.h
 
 /**
  * init_srcu_struct - initialize a sleep-RCU structure
@@ -116,6 +117,7 @@ int srcu_read_lock(struct srcu_struct *s
per_cpu_ptr(sp-per_cpu_ref, smp_processor_id())-c[idx]++;
srcu_barrier();  /* ensure compiler won't misorder critical section. */
preempt_enable();
+   rcu_read_acquire();
return idx;
 }
 
@@ -131,6 +133,7 @@ int srcu_read_lock(struct srcu_struct *s
  */
 void srcu_read_unlock(struct srcu_struct *sp, int idx)
 {
+   rcu_read_release();
preempt_disable();
srcu_barrier();  /* ensure compiler won't misorder critical section. */
per_cpu_ptr(sp-per_cpu_ref, smp_processor_id())-c[idx]--;

 Signed-off-by: Peter Zijlstra [EMAIL PROTECTED]
 ---
  include/linux/lockdep.h  |7 +++
  include/linux/rcupdate.h |   12 
  kernel/rcupdate.c|8 
  3 files changed, 27 insertions(+)
 
 Index: linux-2.6/include/linux/lockdep.h
 ===
 --- linux-2.6.orig/include/linux/lockdep.h
 +++ linux-2.6/include/linux/lockdep.h
 @@ -252,6 +252,13 @@ extern void lockdep_init_map(struct lock
struct lock_class_key *key, int subclass);
 
  /*
 + * To initialize a lockdep_map statically use this macro.
 + * Note that _name must not be NULL.
 + */
 +#define STATIC_LOCKDEP_MAP_INIT(_name, _key) \
 + { .name = (_name), .key = (void *)(_key), }
 +
 +/*
   * Reinitialize a lock key - for cases where there is special locking or
   * special initialization of locks so that the validator gets the scope
   * of dependencies wrong: they are either too broad (they need a class-split)
 Index: linux-2.6/include/linux/rcupdate.h
 ===
 --- linux-2.6.orig/include/linux/rcupdate.h
 +++ linux-2.6/include/linux/rcupdate.h
 @@ -41,6 +41,7 @@
  #include linux/percpu.h
  #include linux/cpumask.h
  #include linux/seqlock.h
 +#include linux/lockdep.h
 
  /**
   * struct rcu_head - callback structure for use with RCU
 @@ -133,6 +134,15 @@ static inline void rcu_bh_qsctr_inc(int 
  extern int rcu_pending(int cpu);
  extern int rcu_needs_cpu(int cpu);
 
 +#ifdef CONFIG_DEBUG_LOCK_ALLOC
 +extern struct lockdep_map rcu_lock_map;
 +# define rcu_read_acquire()  lock_acquire(rcu_lock_map, 0, 0, 2, 1, 
 _THIS_IP_)
 +# define rcu_read_release()  lock_release(rcu_lock_map, 1, _THIS_IP_)
 +#else
 +# define rcu_read_acquire()  do { } while (0)
 +# define rcu_read_release()  do { } while (0)
 +#endif
 +
  /**
   * rcu_read_lock - mark the beginning of an RCU read-side critical section.
   *
 @@ -166,6 +176,7 @@ extern int rcu_needs_cpu(int cpu);
   do { \
   preempt_disable(); \
   __acquire(RCU); \
 + rcu_read_acquire(); \
   } while(0)
 
  /**
 @@ -175,6 +186,7 @@ extern int rcu_needs_cpu(int cpu);
   */
  #define rcu_read_unlock() \
   do { \
 + rcu_read_release(); \
   __release(RCU); \
   preempt_enable(); \
   } while(0)
 @@ -216,6 +218,7 @@ extern struct lockdep_map rcu_lock_map;
   do { \
   local_bh_disable(); \
   __acquire(RCU_BH); \
 + rcu_read_acquire(); \
   } while(0)
 
  /*
 @@ -225,6 +228,7 @@ extern struct lockdep_map rcu_lock_map;
   */
  #define rcu_read_unlock_bh() \
   do { \
 + rcu_read_release(); \
   __release(RCU_BH); \
   local_bh_enable(); \
   } while(0)
 Index: linux-2.6/kernel/rcupdate.c
 ===
 --- linux-2.6.orig/kernel/rcupdate.c
 +++ linux-2.6/kernel/rcupdate.c
 @@ -48,6 +48,14 @@
  #include linux/cpu.h
  #include linux/mutex.h
 
 +#ifdef CONFIG_DEBUG_LOCK_ALLOC
 +static struct lock_class_key rcu_lock_key;
 +struct lockdep_map rcu_lock_map =
 + STATIC_LOCKDEP_MAP_INIT(rcu_read_lock, rcu_lock_key);
 +
 +EXPORT_SYMBOL_GPL(rcu_lock_map);
 +#endif
 +
  /* Definition for rcupdate control block. */
  static struct rcu_ctrlblk rcu_ctrlblk = {
   .cur = -300,