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

[Oleg Nesterov]

On 03/04, Michel Lespinasse wrote:
>
> In lockdep.h, the spinlock/mutex/rwsem/rwlock/lock_map acquire macros
> have different definitions based on the value of CONFIG_PROVE_LOCKING.
> We have separate ifdefs for each of these definitions, which seems
> redundant.

You know, I should not even try to comment this patch. I never really
understood this magic, and I forgot everything I knew.

But I like this patch because it makes lockdep.h more readable to me,
so I hope it is correct ;)

One question,

> +#ifdef CONFIG_PROVE_LOCKING
> + #define lock_acquire_exclusive(l, s, t, n, i)		lock_acquire(l, s, t, 0, 2, n, i)
> + #define lock_acquire_shared(l, s, t, n, i)		lock_acquire(l, s, t, 1, 2, n, i)
> + #define lock_acquire_shared_recursive(l, s, t, n, i)	lock_acquire(l, s, t, 2, 2, n, i)
>  #else
> -# define spin_acquire(l, s, t, i)		do { } while (0)
> -# define spin_release(l, n, i)			do { } while (0)
> + #define lock_acquire_exclusive(l, s, t, n, i)		lock_acquire(l, s, t, 0, 1, n, i)
> + #define lock_acquire_shared(l, s, t, n, i)		lock_acquire(l, s, t, 1, 1, n, i)
> + #define lock_acquire_shared_recursive(l, s, t, n, i)	lock_acquire(l, s, t, 2, 1, n, i)
>  #endif

Do we really need this ifdef? Can't we pass check == 2 unconditionally?

__lock_acquire() does:

	if (!prove_locking)
		check = 1;

anyway, and without CONFIG_PROVE_LOCKING prove_locking == 0.

And every time I need to look into lockdep.h these hardcoded constants
confuse me, and of course it is not possible to remember if this particular
"1" means "read" or "check". Imho it would be nice to add some symbolic
names. But this is offtopic.

Oleg.

--
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/