Re: sparse context warning problem ...

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]

On Sat, 2008-05-10 at 20:18 -0700, David Brownell wrote:
> On Saturday 10 May 2008, Johannes Berg wrote:
> > This is probably my mistake.
> > 
> > However, I took __releases and __acquires to mean that this function
> > *changed* the context, doing both doesn't really make much sense. I
> > think the function should actually be declared
> > 
> > static void
> > finish_urb(...)
> > __requires(ohci->lock)
> > {...}
> > 
> > where __requires is (for sparse) defined as
> > 
> > #define __requires(x) __attribute__((context(x,1,1)))
> 
> ISTR suggesting special syntax for this to Linus (this was way
> back when "sparse" was just starting) and he wanted to just do
> it by having those two attributes.
> 
> So at this point, I'd want to see the regression fixed (and the
> tests updated to avoid this in the future) before exploring any
> alternative syntax for kernel annotations.

Yeah, true, on the other hand, if/when Josh merges my other patches then
most kernel annotations will have to be changed anyway because up to
before the patch that is already in sparse wouldn't flag this:

spin_lock(&lock);
rcu_read_unlock();

because it didn't care *at all* about the expression inside __acquire()
and thus a lot of usage crept in that isn't really usable.

With my future patches, I'm even binding the symbols, so that

void spin_lock(spinlock_t l) __acquire(l);

will flag errors with

spin_lock(&a);
spin_unlock(&b);

while right now it would just treat them both as the context "l".

So I expect that it will not be possible to not "regress" in that sense
because the current annotations are simply messed up since sparse didn't
care about the name inside __acquire()/__release() at all!


However, it's probably fairly easy right now to treat both of them as
being merged together which seems the only sensible things to do anyway
if such a situation is encountered. I'll look into it.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]