2006-08-25 00:48:52

by Josh Triplett

[permalink] [raw]
Subject: [PATCH] Pass sparse the lock expression given to lock annotations

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 <[email protected]>
---
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
[email protected] , available at
http://marc.theaimsgroup.com/[email protected] ).

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



2006-08-25 04:12:06

by Andrew Morton

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

On Thu, 24 Aug 2006 17:48:56 -0700
Josh Triplett <[email protected]> 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?

2006-08-25 15:53:50

by Josh Triplett

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

On Thu, 2006-08-24 at 21:05 -0700, Andrew Morton wrote:
> On Thu, 24 Aug 2006 17:48:56 -0700
> Josh Triplett <[email protected]> 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


2006-08-25 17:46:40

by Andrew Morton

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

On Fri, 25 Aug 2006 08:53:53 -0700
Josh Triplett <[email protected]> wrote:

> On Thu, 2006-08-24 at 21:05 -0700, Andrew Morton wrote:
> > On Thu, 24 Aug 2006 17:48:56 -0700
> > Josh Triplett <[email protected]> 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.

2006-08-25 18:21:55

by Josh Triplett

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

On Fri, 2006-08-25 at 10:46 -0700, Andrew Morton wrote:
> On Fri, 25 Aug 2006 08:53:53 -0700
> Josh Triplett <[email protected]> wrote:
>
> > On Thu, 2006-08-24 at 21:05 -0700, Andrew Morton wrote:
> > > On Thu, 24 Aug 2006 17:48:56 -0700
> > > Josh Triplett <[email protected]> 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


2006-08-31 17:49:08

by Josh Triplett

[permalink] [raw]
Subject: [PATCH v2] Pass sparse the lock expression given to lock annotations

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 <[email protected]>
---
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