On Tuesday 13 May 2008, Johannes Berg wrote: > 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, I used the OHCI example because it was a clear and readily available/reproducible example of how this is a regression; but I came across the problem with a different driver. In that driver, I just made sure that the strings were now identical ... and the failures still came up with "sparse". That's on a UP build. (Albeit with PREEMPT and all the debug options available ... since I'm debugging!) > 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. Yeah, well the lock being acquired or released *IS* "ohci->lock", but the spinlock calls don't take the lock, they take pointers to it! If you were to argue that understanding pointers like that is a lot to demand of "sparse", I might agree. But that won't change the fact that locks themselves are not pointers to locks. ;) > 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. This still doesn't quite add up, though... - Dave -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
