linux-kernel - Re: [PATCH 1/2] lockdep: introduce lock_acquire

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 5 Mar 2013 18:06:42 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Michel Lespinasse <walken@...gle.com>
Cc:	linux-kernel@...r.kernel.org,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
	paulmck@...ux.vnet.ibm.com, Rusty Russell <rusty@...tcorp.com.au>,
	rostedt@...dmis.org, tglx@...utronix.de,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH 1/2] lockdep: introduce lock_acquire_exclusive/shared
	helper macros

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/