Re: [RFC] seqlock,lockdep: Add lock primitives to read_seqbegin().

[Peter Zijlstra]

On Sat, 2011-03-26 at 13:12 +0900, Tetsuo Handa wrote:
> @@ -86,6 +86,11 @@ static inline int write_tryseqlock(seqlo
>  static __always_inline unsigned read_seqbegin(const seqlock_t *sl)
>  {
>         unsigned ret;
> +#ifdef CONFIG_PROVE_LOCKING
> +       unsigned long flags;
> +       spin_lock_irqsave(&((seqlock_t *) sl)->lock, flags);
> +       spin_unlock_irqrestore(&((seqlock_t *) sl)->lock, flags);
> +#endif
>  
>  repeat:
>         ret = sl->sequence; 

That isn't the right way, something like the below would do, however
there's a reason this isn't done, we use these primitives from the VDSO
which means they're not permitted to write to any kernel memory at all.

We could sort that by creating a raw_ variant that doesn't get the
annotation, something I think is already done for -rt anyway.


---
 include/linux/seqlock.h |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index e98cd2e..f2bc19c 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -94,6 +94,7 @@ static __always_inline unsigned read_seqbegin(const seqlock_t *sl)
 		cpu_relax();
 		goto repeat;
 	}
+	rwlock_acquire_read(&sl->lock->dep_map, 0, 0, _RET_IP_);
 
 	return ret;
 }
@@ -107,6 +108,8 @@ static __always_inline int read_seqretry(const seqlock_t *sl, unsigned start)
 {
 	smp_rmb();
 
+	rwlock_release(&sl->lock->dep_map, 1, _RET_IP_);
+
 	return unlikely(sl->sequence != start);
 }
 
@@ -120,6 +123,9 @@ static __always_inline int read_seqretry(const seqlock_t *sl, unsigned start)
 
 typedef struct seqcount {
 	unsigned sequence;
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	struct lockdep_map dep_map;
+#endif
 } seqcount_t;
 
 #define SEQCNT_ZERO { 0 }
@@ -148,6 +154,7 @@ static inline unsigned __read_seqcount_begin(const seqcount_t *s)
 		cpu_relax();
 		goto repeat;
 	}
+	rwlock_acquire_read(&sl->dep_map, 0, 0, _RET_IP_);
 	return ret;
 }
 
@@ -183,7 +190,9 @@ static inline unsigned read_seqcount_begin(const seqcount_t *s)
  */
 static inline int __read_seqcount_retry(const seqcount_t *s, unsigned start)
 {
-	return unlikely(s->sequence != start);
+	int ret = unlikely(s->sequence != start);
+	rwlock_release(&sl->dep_map, 1, _RET_IP_);
+	return ret;
 }
 
 /**
@@ -210,6 +219,7 @@ static inline int read_seqcount_retry(const seqcount_t *s, unsigned start)
  */
 static inline void write_seqcount_begin(seqcount_t *s)
 {
+	rwlock_acquire(&sl->dep_map, 0, 0, _RET_IP_);
 	s->sequence++;
 	smp_wmb();
 }
@@ -218,6 +228,7 @@ static inline void write_seqcount_end(seqcount_t *s)
 {
 	smp_wmb();
 	s->sequence++;
+	rwlock_release(&sl->dep_map, 1, _RET_IP_);
 }
 
 /**


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