Re: [PATCH 1/2] lockdep: introduce lock_acquire_exclusive/sharedhelper macros

[Michel Lespinasse]

On Tue, Mar 5, 2013 at 7:19 AM, Lai Jiangshan <laijs@xxxxxxxxxxxxxx> wrote:
> I don't like the name lock_acquire_shared_recursive().
> (I mean the name is wrong, ......)

Names are hard :/ Do you have a better suggestion ?

> In the lockdep design, lock_acquire(l, s, t, 2, 2, n, i) is used for
> read-preference locks(rwlock) and all types of RCU. not for "recursive"
> read-preference implies "recursive".

I based my names on the following description from lockdep.h:

 * Values for "read":
 *
 *   0: exclusive (write) acquire
 *   1: read-acquire (no recursion allowed)
 *   2: read-acquire with same-instance recursion allowed

> But the name lock_acquire_shared_recursive() don't tell us it is
> read-preference.
>
> Example if we do have a lock which is write-preference but allow read_lock recursive,
> it will be still deadlock in this way, "recursive" does not help:
>
> cpu0: spin_lock(a);                             recursiveable_read_lock(b)
> cpu1: recursiveable_read_lock(b);               spin_lock(a);
> cpu2:                           write_lock(b);

I think this is a very theorical distinction as 1- we don't have such
locks in linux currently, and 2- lockdep does not currently support
this. That is, lockdep will either report both of these scenarios as
valid or invalid, but you can't tell it to allow the first scenario
while disallowing the second:

read_lock(a); read_lock(a);
(the recursive read lock scenario)

read_lock(a); lock(b); vs lock(b); read_lock(a);
(the lock ordering vs other locks scenario)

So, I would think that since lockdep doesn't make this distinction the
helper macros don't need to either ?

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/