linux-kernel - Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature

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]

Message-ID: <20170228132444.GG3817@X58A-UD3R>
Date:   Tue, 28 Feb 2017 22:24:44 +0900
From:   Byungchul Park <byungchul.park@....com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     mingo@...nel.org, tglx@...utronix.de, walken@...gle.com,
        boqun.feng@...il.com, kirill@...temov.name,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        iamjoonsoo.kim@....com, akpm@...ux-foundation.org,
        npiggin@...il.com
Subject: Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature

On Tue, Feb 28, 2017 at 02:10:12PM +0100, Peter Zijlstra wrote:
> > +#ifdef CONFIG_LOCKDEP_CROSSRELEASE
> > +
> > +#define idx(t)			((t)->xhlock_idx)
> > +#define idx_prev(i)		((i) ? (i) - 1 : MAX_XHLOCKS_NR - 1)
> > +#define idx_next(i)		(((i) + 1) % MAX_XHLOCKS_NR)
> 
> Note that:
> 
> #define idx_prev(i)		(((i) - 1) % MAX_XHLOCKS_NR)
> #define idx_next(i)		(((i) + 1) % MAX_XHLOCKS_NR)
> 
> is more symmetric and easier to understand.

OK. I will do it after forcing MAX_XHLOCKS_NR to be power of 2. Current
value of it is already power of 2 but I need to add comment explaning it.

> > +
> > +/* For easy access to xhlock */
> > +#define xhlock(t, i)		((t)->xhlocks + (i))
> > +#define xhlock_prev(t, l)	xhlock(t, idx_prev((l) - (t)->xhlocks))
> > +#define xhlock_curr(t)		xhlock(t, idx(t))
> 
> So these result in an xhlock pointer
> 
> > +#define xhlock_incr(t)		({idx(t) = idx_next(idx(t));})
> 
> This does not; which is confusing seeing how they share the same
> namespace; also incr is weird.

OK.. Could you suggest a better name? xhlock_adv()? advance_xhlock()?
And.. replace it with a function?