Re: [cpuops cmpxchg double V2 1/4] Generic support forthis_cpu_cmpxchg_double

[Christoph Lameter]

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