Re: Another sparse warning...

[Josh Triplett <josh@freedesktop.org>]

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

Linus Torvalds wrote:
> On Mon, 12 Feb 2007, Christopher Li wrote:
>>>  - it shows the *programmer* that the function is doing somethign 
>>>    "strange" (not really strange, but still: it's basically a fairly 
>>>    readable way that it's doing locking in a weird way).
>> Should the function declare in the header file has that as well?
> 
> Yes.
> 
>> When call one of those functions, it can know that function will change
>> context.  That might be a way to solve the problem that some of the
>> spinlock function is not a inline function at all.
> 
> I thought we did that already. I'm fairly sure I had this working at some 
> point - exactly by having the calls just add up the (known) lock/unlock 
> offsets.

On the other hand, the code currently does not check the context preconditions
of the callee; Sparse *could* check that any function requiring you to hold a
lock (have a context of 1) must only get called from a context in which you
hold that lock (have that context).  Instead, Sparse only checks the context
change, rather than the incoming context value at the call site.  It seems
trivial to add a check for context preconditions, and Linux could then have a
new annotation for __must_hold(lock) (usable on both functions and data), but
that check would also add numerous false positives caused by lack of
annotation, and that problem seems better solved by interprocedural analysis
than pervasive annotation.

Thoughts?

- Josh Triplett



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]