[PATCH] Make spinlock/rwlock annotations more accurate by using parameters, not types

[PATCH] Make spinlock/rwlock annotations more accurate by using parameters, not types

[Josh Triplett]

The lock annotations used on spinlocks and rwlocks currently use
__{acquires,releases}(spinlock_t) and __{acquires,releases}(rwlock_t),
respectively.  This loses the information of which lock actually got acquired
or released, and assumes a different type for the parameter of __acquires and
__releases than the rest of the kernel.  While the current implementations of
__acquires and __releases throw away their argument, this will not always
remain the case.  Change this to use the lock parameter instead, to preserve
this information and increase consistency in usage of __acquires and
__releases.

Signed-off-by: Josh Triplett <josh@freedesktop.org>
---
 include/linux/spinlock_api_smp.h |   50 +++++++++++++++++++-------------------
 1 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/include/linux/spinlock_api_smp.h b/include/linux/spinlock_api_smp.h
index b2c4f82..8828b81 100644
--- a/include/linux/spinlock_api_smp.h
+++ b/include/linux/spinlock_api_smp.h
@@ -19,41 +19,41 @@ int in_lock_functions(unsigned long addr
 
 #define assert_spin_locked(x)	BUG_ON(!spin_is_locked(x))
 
-void __lockfunc _spin_lock(spinlock_t *lock)		__acquires(spinlock_t);
+void __lockfunc _spin_lock(spinlock_t *lock)		__acquires(lock);
 void __lockfunc _spin_lock_nested(spinlock_t *lock, int subclass)
-							__acquires(spinlock_t);
-void __lockfunc _read_lock(rwlock_t *lock)		__acquires(rwlock_t);
-void __lockfunc _write_lock(rwlock_t *lock)		__acquires(rwlock_t);
-void __lockfunc _spin_lock_bh(spinlock_t *lock)		__acquires(spinlock_t);
-void __lockfunc _read_lock_bh(rwlock_t *lock)		__acquires(rwlock_t);
-void __lockfunc _write_lock_bh(rwlock_t *lock)		__acquires(rwlock_t);
-void __lockfunc _spin_lock_irq(spinlock_t *lock)	__acquires(spinlock_t);
-void __lockfunc _read_lock_irq(rwlock_t *lock)		__acquires(rwlock_t);
-void __lockfunc _write_lock_irq(rwlock_t *lock)		__acquires(rwlock_t);
+							__acquires(lock);
+void __lockfunc _read_lock(rwlock_t *lock)		__acquires(lock);
+void __lockfunc _write_lock(rwlock_t *lock)		__acquires(lock);
+void __lockfunc _spin_lock_bh(spinlock_t *lock)		__acquires(lock);
+void __lockfunc _read_lock_bh(rwlock_t *lock)		__acquires(lock);
+void __lockfunc _write_lock_bh(rwlock_t *lock)		__acquires(lock);
+void __lockfunc _spin_lock_irq(spinlock_t *lock)	__acquires(lock);
+void __lockfunc _read_lock_irq(rwlock_t *lock)		__acquires(lock);
+void __lockfunc _write_lock_irq(rwlock_t *lock)		__acquires(lock);
 unsigned long __lockfunc _spin_lock_irqsave(spinlock_t *lock)
-							__acquires(spinlock_t);
+							__acquires(lock);
 unsigned long __lockfunc _read_lock_irqsave(rwlock_t *lock)
-							__acquires(rwlock_t);
+							__acquires(lock);
 unsigned long __lockfunc _write_lock_irqsave(rwlock_t *lock)
-							__acquires(rwlock_t);
+							__acquires(lock);
 int __lockfunc _spin_trylock(spinlock_t *lock);
 int __lockfunc _read_trylock(rwlock_t *lock);
 int __lockfunc _write_trylock(rwlock_t *lock);
 int __lockfunc _spin_trylock_bh(spinlock_t *lock);
-void __lockfunc _spin_unlock(spinlock_t *lock)		__releases(spinlock_t);
-void __lockfunc _read_unlock(rwlock_t *lock)		__releases(rwlock_t);
-void __lockfunc _write_unlock(rwlock_t *lock)		__releases(rwlock_t);
-void __lockfunc _spin_unlock_bh(spinlock_t *lock)	__releases(spinlock_t);
-void __lockfunc _read_unlock_bh(rwlock_t *lock)		__releases(rwlock_t);
-void __lockfunc _write_unlock_bh(rwlock_t *lock)	__releases(rwlock_t);
-void __lockfunc _spin_unlock_irq(spinlock_t *lock)	__releases(spinlock_t);
-void __lockfunc _read_unlock_irq(rwlock_t *lock)	__releases(rwlock_t);
-void __lockfunc _write_unlock_irq(rwlock_t *lock)	__releases(rwlock_t);
+void __lockfunc _spin_unlock(spinlock_t *lock)		__releases(lock);
+void __lockfunc _read_unlock(rwlock_t *lock)		__releases(lock);
+void __lockfunc _write_unlock(rwlock_t *lock)		__releases(lock);
+void __lockfunc _spin_unlock_bh(spinlock_t *lock)	__releases(lock);
+void __lockfunc _read_unlock_bh(rwlock_t *lock)		__releases(lock);
+void __lockfunc _write_unlock_bh(rwlock_t *lock)	__releases(lock);
+void __lockfunc _spin_unlock_irq(spinlock_t *lock)	__releases(lock);
+void __lockfunc _read_unlock_irq(rwlock_t *lock)	__releases(lock);
+void __lockfunc _write_unlock_irq(rwlock_t *lock)	__releases(lock);
 void __lockfunc _spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags)
-							__releases(spinlock_t);
+							__releases(lock);
 void __lockfunc _read_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
-							__releases(rwlock_t);
+							__releases(lock);
 void __lockfunc _write_unlock_irqrestore(rwlock_t *lock, unsigned long flags)
-							__releases(rwlock_t);
+							__releases(lock);
 
 #endif /* __LINUX_SPINLOCK_API_SMP_H */
-- 
1.4.1.1

Re: [PATCH] Make spinlock/rwlock annotations more accurate by using parameters, not types

[Andrew Morton]

On Tue, 22 Aug 2006 17:51:38 -0700
Josh Triplett <josht@us.ibm.com> wrote:

> The lock annotations used on spinlocks and rwlocks currently use
> __{acquires,releases}(spinlock_t) and __{acquires,releases}(rwlock_t),
> respectively.  This loses the information of which lock actually got acquired
> or released, and assumes a different type for the parameter of __acquires and
> __releases than the rest of the kernel.  While the current implementations of
> __acquires and __releases throw away their argument, this will not always
> remain the case.

It won't?  Why, what will happen?

Re: [PATCH] Make spinlock/rwlock annotations more accurate by using parameters, not types

[Josh Triplett]

On Tue, 2006-08-22 at 20:53 -0700, Andrew Morton wrote:
> On Tue, 22 Aug 2006 17:51:38 -0700
> Josh Triplett <josht@us.ibm.com> wrote:
> 
> > The lock annotations used on spinlocks and rwlocks currently use
> > __{acquires,releases}(spinlock_t) and __{acquires,releases}(rwlock_t),
> > respectively.  This loses the information of which lock actually got acquired
> > or released, and assumes a different type for the parameter of __acquires and
> > __releases than the rest of the kernel.  While the current implementations of
> > __acquires and __releases throw away their argument, this will not always
> > remain the case.
> 
> It won't?  Why, what will happen?

See http://marc.theaimsgroup.com/?l=linux-sparse&m=115644727723278&w=2
(Message-ID: 1156447273.3418.34.camel@josh-work.beaverton.ibm.com); with
that patch, sparse can track a context expression per context.  I plan
to make sparse use that context expression to track contexts
independently to improve the correctness and usefulness of context
warnings.  The Linux patch "Pass sparse the lock expression given to
lock annotations" that I just sent (Message-ID:
1156466936.3418.58.camel@josh-work.beaverton.ibm.com) changes
__acquires, __releases, __acquire, and __release to stop ignoring the
lock expression, and pass it to sparse.

- Josh Triplett