[PATCH] Pass sparse the lock expression given to lock annotations

[PATCH] Pass sparse the lock expression given to lock annotations

[Josh Triplett]

The lock annotation macros __acquires, __releases, __acquire, and __release
all currently throw the lock expression passed as an argument.  Now that
sparse can parse __context__ and __attribute__((context)) with a context
expression, pass the lock expression down to sparse as the context expression.

Signed-off-by: Josh Triplett <josh@freedesktop.org>
---
This patch depends on the Linux patch "Make spinlock/rwlock annotations
more accurate by using parameters, not types" (currently in -mm as
make-spinlock-rwlock-annotations-more-accurate-by-using.patch) and on
the sparse patch "Parse and track multiple contexts by
expression" (Message-ID
1156447273.3418.34.camel@josh-work.beaverton.ibm.com , available at
http://marc.theaimsgroup.com/?i=1156447273.3418.34.camel@josh-work.beaverton.ibm.com ).

Starting from a base of 2.6.18-rc4 plus
make-spinlock-rwlock-annotations-more-accurate-by-using.patch, using
allyesconfig, I tested sparse -Wcontext with and without this patch, and
confirmed that it generates identical output.

 include/linux/compiler.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index b3963cf..2ed6528 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -10,10 +10,10 @@ # define __safe		__attribute__((safe))
 # define __force	__attribute__((force))
 # define __nocast	__attribute__((nocast))
 # define __iomem	__attribute__((noderef, address_space(2)))
-# define __acquires(x)	__attribute__((context(0,1)))
-# define __releases(x)	__attribute__((context(1,0)))
-# define __acquire(x)	__context__(1)
-# define __release(x)	__context__(-1)
+# define __acquires(x)	__attribute__((context(x,0,1)))
+# define __releases(x)	__attribute__((context(x,1,0)))
+# define __acquire(x)	__context__(x,1)
+# define __release(x)	__context__(x,-1)
 # define __cond_lock(x,c)	((c) ? ({ __acquire(x); 1; }) : 0)
 extern void __chk_user_ptr(void __user *);
 extern void __chk_io_ptr(void __iomem *);
-- 
1.4.1.1

Re: [PATCH] Pass sparse the lock expression given to lock annotations

[Andrew Morton]

On Thu, 24 Aug 2006 17:48:56 -0700
Josh Triplett <josht@us.ibm.com> wrote:

> The lock annotation macros __acquires, __releases, __acquire, and __release
> all currently throw the lock expression passed as an argument.  Now that
> sparse can parse __context__ and __attribute__((context)) with a context
> expression, pass the lock expression down to sparse as the context expression.

What is the dependency relationship between your kernel changes and your
proposed change to sparse?

Re: [PATCH] Pass sparse the lock expression given to lock annotations

[Josh Triplett]

On Thu, 2006-08-24 at 21:05 -0700, Andrew Morton wrote:
> On Thu, 24 Aug 2006 17:48:56 -0700
> Josh Triplett <josht@us.ibm.com> wrote:
> 
> > The lock annotation macros __acquires, __releases, __acquire, and __release
> > all currently throw the lock expression passed as an argument.  Now that
> > sparse can parse __context__ and __attribute__((context)) with a context
> > expression, pass the lock expression down to sparse as the context expression.
> 
> What is the dependency relationship between your kernel changes and your
> proposed change to sparse?

Sparse with my multi-context patch will continue to parse versions of
the kernel without this kernel patch, since I made the context
expression optional in sparse.  Versions of sparse without my
multi-context patch will not parse kernels with this kernel patch (since
previous versions of sparse will not support the extra argument).  The
same dependency relationship has held true with past additions to sparse
and the kernel; furthermore, that dependency relationship often exists
anyway due to the use of new GCC extensions in the kernel which require
changes to the sparse parser, such as __builtin_types_compatible_p,
__builtin_extract_return_addr, __builtin_va_copy, and
__attribute__((no_instrument_function)).

- Josh Triplett

Re: [PATCH] Pass sparse the lock expression given to lock annotations

[Andrew Morton]

On Fri, 25 Aug 2006 08:53:53 -0700
Josh Triplett <josht@us.ibm.com> wrote:

> On Thu, 2006-08-24 at 21:05 -0700, Andrew Morton wrote:
> > On Thu, 24 Aug 2006 17:48:56 -0700
> > Josh Triplett <josht@us.ibm.com> wrote:
> > 
> > > The lock annotation macros __acquires, __releases, __acquire, and __release
> > > all currently throw the lock expression passed as an argument.  Now that
> > > sparse can parse __context__ and __attribute__((context)) with a context
> > > expression, pass the lock expression down to sparse as the context expression.
> > 
> > What is the dependency relationship between your kernel changes and your
> > proposed change to sparse?
> 
> Sparse with my multi-context patch will continue to parse versions of
> the kernel without this kernel patch, since I made the context
> expression optional in sparse.  Versions of sparse without my
> multi-context patch will not parse kernels with this kernel patch (since
> previous versions of sparse will not support the extra argument).

OK.  Is this patch the only one which will break current sparse versions? 
If not, please identify the others.

I'll keep the non-back-compatible kernel patches in -mm until you've
informed me that a suitable version of sparse has been released.  Then we
can include that sparse version number in the kernel changelogs so things
are nice and organised.

Re: [PATCH] Pass sparse the lock expression given to lock annotations

[Josh Triplett]

On Fri, 2006-08-25 at 10:46 -0700, Andrew Morton wrote:
> On Fri, 25 Aug 2006 08:53:53 -0700
> Josh Triplett <josht@us.ibm.com> wrote:
> 
> > On Thu, 2006-08-24 at 21:05 -0700, Andrew Morton wrote:
> > > On Thu, 24 Aug 2006 17:48:56 -0700
> > > Josh Triplett <josht@us.ibm.com> wrote:
> > > 
> > > > The lock annotation macros __acquires, __releases, __acquire, and __release
> > > > all currently throw the lock expression passed as an argument.  Now that
> > > > sparse can parse __context__ and __attribute__((context)) with a context
> > > > expression, pass the lock expression down to sparse as the context expression.
> > > 
> > > What is the dependency relationship between your kernel changes and your
> > > proposed change to sparse?
> > 
> > Sparse with my multi-context patch will continue to parse versions of
> > the kernel without this kernel patch, since I made the context
> > expression optional in sparse.  Versions of sparse without my
> > multi-context patch will not parse kernels with this kernel patch (since
> > previous versions of sparse will not support the extra argument).
> 
> OK.  Is this patch the only one which will break current sparse versions? 
> If not, please identify the others.

Only this patch will break current sparse versions.

> I'll keep the non-back-compatible kernel patches in -mm until you've
> informed me that a suitable version of sparse has been released.  Then we
> can include that sparse version number in the kernel changelogs so things
> are nice and organised.

Sounds good.  Since sparse doesn't really have "releases", just the
kernel.org GIT tree, I'll let you know when it gets committed and supply
an updated patch with the appropriate GIT revision in the commit
message.

- Josh Triplett

[PATCH v2] Pass sparse the lock expression given to lock annotations

[Josh Triplett]

The lock annotation macros __acquires, __releases, __acquire, and __release
all currently throw away the lock expression passed as an argument.  Now that
sparse can parse __context__ and __attribute__((context)) with a context
expression, pass the lock expression down to sparse as the context expression.
This requires a version of sparse from GIT commit
37475a6c1c3e66219e68d912d5eb833f4098fd72 or later.

Signed-off-by: Josh Triplett <josh@freedesktop.org>
---
The current sparse GIT tree now includes my patch for parsing a context
expression; updated commit message to reference the GIT commit in the
sparse tree, and to fix a typo.  Please replace
"pass-sparse-the-lock-expression-given-to-lock-annotations.patch" in the
current -mm tree with this patch.

 include/linux/compiler.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index b3963cf..2ed6528 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -10,10 +10,10 @@ # define __safe		__attribute__((safe))
 # define __force	__attribute__((force))
 # define __nocast	__attribute__((nocast))
 # define __iomem	__attribute__((noderef, address_space(2)))
-# define __acquires(x)	__attribute__((context(0,1)))
-# define __releases(x)	__attribute__((context(1,0)))
-# define __acquire(x)	__context__(1)
-# define __release(x)	__context__(-1)
+# define __acquires(x)	__attribute__((context(x,0,1)))
+# define __releases(x)	__attribute__((context(x,1,0)))
+# define __acquire(x)	__context__(x,1)
+# define __release(x)	__context__(x,-1)
 # define __cond_lock(x,c)	((c) ? ({ __acquire(x); 1; }) : 0)
 extern void __chk_user_ptr(void __user *);
 extern void __chk_io_ptr(void __iomem *);
-- 
1.4.2.ga444