Re: On some configs, sparse spinlock balance checking is broken

From: Roland Dreier
Date: Wed Jan 17 2007 - 10:38:26 EST


> i think the right way to fix it might be to define a _spin_unlock()
> within those #ifdef branches, and then to define spin_lock as:
>
> static inline void spin_lock(spinlock_t *lock) __acquires(lock)

I tried a similar approach, but what got me was that sparse doesn't
pay attention to the "__acquires()" annotation there. However I now
realized that putting "__acquire()" inside the implementation of the
function (which sparse can see for inline functions) actually works.

And actually the lock stuff is OK, since it's not inlined -- it's the
unlock stuff that goes directly to the __raw versions. But something
like the following works for me; does it look OK to you?

---

diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index 94b767d..8ec4142 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -228,15 +228,45 @@ do { \
# define read_unlock_irq(lock) _read_unlock_irq(lock)
# define write_unlock_irq(lock) _write_unlock_irq(lock)
#else
-# define spin_unlock(lock) __raw_spin_unlock(&(lock)->raw_lock)
-# define read_unlock(lock) __raw_read_unlock(&(lock)->raw_lock)
-# define write_unlock(lock) __raw_write_unlock(&(lock)->raw_lock)
-# define spin_unlock_irq(lock) \
- do { __raw_spin_unlock(&(lock)->raw_lock); local_irq_enable(); } while (0)
-# define read_unlock_irq(lock) \
- do { __raw_read_unlock(&(lock)->raw_lock); local_irq_enable(); } while (0)
-# define write_unlock_irq(lock) \
- do { __raw_write_unlock(&(lock)->raw_lock); local_irq_enable(); } while (0)
+static inline void spin_unlock(spinlock_t *lock)
+{
+ __release(lock);
+ __raw_spin_unlock(&(lock)->raw_lock);
+}
+
+static inline void read_unlock(rwlock_t *lock)
+{
+ __release(lock);
+ __raw_read_unlock(&(lock)->raw_lock);
+}
+
+static inline void write_unlock(rwlock_t *lock)
+{
+ __release(lock);
+ __raw_write_unlock(&(lock)->raw_lock);
+}
+
+static inline void spin_unlock_irq(spinlock_t *lock)
+{
+ __release(lock);
+ __raw_spin_unlock(&(lock)->raw_lock);
+ local_irq_enable();
+}
+
+static inline void read_unlock_irq(rwlock_t *lock)
+{
+ __release(lock);
+ __raw_read_unlock(&(lock)->raw_lock);
+ local_irq_enable();
+}
+
+static inline void write_unlock_irq(rwlock_t *lock)
+{
+ __release(lock);
+ __raw_write_unlock(&(lock)->raw_lock);
+ local_irq_enable();
+}
+
#endif

#define spin_unlock_irqrestore(lock, flags) \
-
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/