Re: sparse context warning problem ...

[Johannes Berg <johannes@sipsolutions.net>]

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


> >   static void
> >   finish_urb(struct ohci_hcd *ohci, struct urb *urb, int status)
> >   __releases(ohci->lock)
> >   __acquires(ohci->lock)

> > But current versions of "sparse" complain (wrongly):
> > 
> >   drivers/usb/host/ohci-q.c:66:2: warning: context imbalance in 'finish_urb': __context__ statement expected different context
> >   drivers/usb/host/ohci-q.c:66:2:    context '<noident>': wanted >= 0, got -1


> 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)))

Actually, it turns out my analysis was completely wrong.

This is exactly the issue I pointed out in my other mail. You have:

__acquires(ohci->lock)
           ^^^^^^^^^^
but, on the other hand:

spin_unlock (&ohci->lock);
             ^

I think you can fix this particular case by adding the & in the
__acquires(), but that will only work for UP, for actual spinlocks my
other patches will be needed, because w/o my patches sparse will, on
SMP, not be able to see that

void __lockfunc _spin_lock(spinlock_t *lock)            __acquires(lock);

means to lock "&ohci->lock" when doing "spin_lock(&ohci->lock);" but
will treat it as locking the abstractly-named "lock" context, while on
UP/no-preempt the "spin_lock" macro is expanded by the preprocessor and
you will get "&ohci->lock" as the expression.

Ultimately, this whole problem comes from the fact that sparse accepted
adding an expression, documented it, but never complained if they
slightly mismatched as above.

johannes

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