Johannes Berg wrote:
>>> static void *dev_mc_seq_start(struct seq_file *seq, loff_t * pos)
>>> __acquires(dev_base_lock)
>>> {
>>> [...]
>>> __acquire__(dev_base_lock);
>>> read_lock(&dev_base_lock);
>>> [...]
>>> }
>> I don't understand why you count this as wrong. read_lock should
>> handle acquiring the context itself, either via an __acquires
>> annotation if written as a function, or via an __acquire__ statement
>> if written as a macro. Callers of read_lock shouldn't need to
>> explicitly call __acquire__.
>
> Well, the question is how you want to name things. What I did is that
> the context identifier is just a name and bears no other relation to the
> code. Hence, read_lock() can't say that it acquires 'dev_base_lock'
> because it doesn't know what the name should be.
>
> With a slightly different sparse patch than mine you could declare
> read_lock() something like this:
>
> #define read_lock(x) do { \
> __acquire__(x); \
> __read_lock((x)); \
> } while (0)
>
> but then you'd have different names everywhere, say somebody passed
> '&dev_base_lock' and somebody else used
>
> readlock_t *dbl = &dev_base_lock;
> read_lock(dbl)
>
> and you'd end up with one context named "dbl" and another one named
> "&dev_base_lock".
That might still work, depending on how consistently kernel code uses
the same argument. If you suggest explicitly changing calls to
read_lock to call __acquire__ with the appropriate context, it might
prove equally easy to make the argument of read_lock that context.
> If you have suggestions on how to solve this I'm all ears, so far I
> decided it wasn't worth it and opted for explicitly naming all the
> contexts.
>
> (with my patch the above would just create a context called "x")
That doesn't make sense to me; after preprocessing, x no longer
exists, so I don't see how you could pick up the identifier "x". I
can understand that you might pick up two different expressions in
place of x which you can't easily compare, though.
And as for how to solve it: I think alias analysis might work.
- Josh Triplett
Attachment:
signature.asc
Description: OpenPGP digital signature
