> > 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:
signature.asc
Description: This is a digitally signed message part
