linux-kernel - Re: [cpuops cmpxchg double V2 1/4] Generic support for this_cpu_cmpxchg

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: <alpine.DEB.2.00.1101070907060.3014@router.home>
Date:	Fri, 7 Jan 2011 09:15:05 -0600 (CST)
From:	Christoph Lameter <cl@...ux.com>
To:	"H. Peter Anvin" <hpa@...or.com>
cc:	Tejun Heo <tj@...nel.org>, akpm@...ux-foundation.org,
	Pekka Enberg <penberg@...helsinki.fi>,
	linux-kernel@...r.kernel.org,
	Eric Dumazet <eric.dumazet@...il.com>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Subject: Re: [cpuops cmpxchg double V2 1/4] Generic support for
 this_cpu_cmpxchg_double

On Thu, 6 Jan 2011, H. Peter Anvin wrote:

> On 01/06/2011 12:45 PM, Christoph Lameter wrote:
> > Introduce this_cpu_cmpxchg_double. this_cpu_cmpxchg_double() allows the
> > comparision between two consecutive words and replaces them if there is
> > a match.
> >
> > 	bool this_cpu_cmpxchg_double(pcp1, pcp2,
> > 		old_word1, old_word2, new_word1, new_word2)
> >
> > this_cpu_cmpxchg_double does not return the old value (difficult since
> > there are two words) but a boolean indicating if the operation was
> > successful.
> >
> > The first percpu variable must be double word aligned!
>
> I really truly hate this interface.  The whole notion of passing two
> pointers where only one is really used, is just painful.

Well the second cpu variable is just there for correctness checking. That
way we can verify that the types are compatible for the second word and
that the second word was properly placed relative to the first one. It
also helps the reader in the source because it shows explicitly which two
words are modified by the operation.

If you look at the prior patch series at the use case you will see that
s->cpu_slab->tid was not referred to in the cmpxchg operation but
implicitly modified. That is bad and was the initial motivation to require
both to be specified.

During debugging I had a couple of problems with the way the compiler
placed those two fields that only became evident after painful debugging
sessions. With the checks possible only because both are specifiec there
is an explicit failure if either of the two cpu variables is specified in
such a way that the cmpxchg32b cannot work.

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