Re: "unexpected unlock" when unlocking, conditional, lock in loop

[Josh Triplett <josh@joshtriplett.org>]

On Sun, Oct 07, 2012 at 07:49:25AM -0500, Ed Cashin wrote:
> On Oct 6, 2012, at 10:39 PM, Josh Triplett wrote:
> 
> > On Sat, Oct 06, 2012 at 08:56:57PM -0500, Ed Cashin wrote:
> ...
> >> OK.  From the sparse man page section on context, along with
> >> include/linux/compiler.h, it sounds like the way to do exactly that 
> >> would be something unusual:
> >> 
> >> int demofn(void) __attribute__((context(&lk,1,1)))
> >> 
> >> ... but using that in demo.c causes sparse to warn me that it's 
> >> ignoring that attribute, so I doubt that can be what you mean.
> > 
> > I did mean precisely that; I don't know why Sparse complains about that
> > syntax.
> 
> Maybe there's a header I need.  Searching with cscope and ctags for definitions of "context" doesn't seem to be the right kind of searching.
> 
> The complaint looks like:
> 
>   CC [M]  drivers/block/aoe/demo.o
> drivers/block/aoe/demo.c:9: warning: `context' attribute directive ignored
> drivers/block/aoe/demo.c:9: error: expected `,' or `;' before `{' token
> make[1]: *** [drivers/block/aoe/demo.o] Error 1
> make: *** [drivers/block/aoe/aoe.ko] Error 2

Oh, that complaint doesn't come from Sparse; that comes from GCC, since
GCC doesn't understand the context attribute.  Look at
include/linux/compiler.h; it has wrapper macros for the various Sparse
attributes, and defines them to nothing when not compiling with Sparse.
If you want to use the context attribute to denote a lock held through a
function, you need a patch like this:

From 0b862fc1a131a874d157420e9443f16a714596ef Mon Sep 17 00:00:00 2001
From: Josh Triplett <josh@joshtriplett.org>
Date: Sun, 7 Oct 2012 12:41:13 -0700
Subject: [PATCH] linux/compiler.h: Add __must_hold macro for functions called with a lock held

linux/compiler.h has macros to denote functions that acquire or release
locks, but not to denote functions called with a lock held that return
with the lock still held.  Add a __must_hold macro to cover this case.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 include/linux/compiler.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index f430e41..b121554 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -10,6 +10,7 @@
 # define __force	__attribute__((force))
 # define __nocast	__attribute__((nocast))
 # define __iomem	__attribute__((noderef, address_space(2)))
+# define __must_hold(x)	__attribute__((context(x,1,1)))
 # define __acquires(x)	__attribute__((context(x,0,1)))
 # define __releases(x)	__attribute__((context(x,1,0)))
 # define __acquire(x)	__context__(x,1)
@@ -33,6 +34,7 @@ extern void __chk_io_ptr(const volatile void __iomem *);
 # define __chk_user_ptr(x) (void)0
 # define __chk_io_ptr(x) (void)0
 # define __builtin_warning(x, y...) (1)
+# define __must_hold(x)
 # define __acquires(x)
 # define __releases(x)
 # define __acquire(x) (void)0