Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed

[Paul Burton]

Hi Guenter,

On Wed, Oct 31, 2018 at 12:52:18PM -0700, Guenter Roeck wrote:
> +/*
> + * Generic version of __cmpxchg_u64, to be used for cmpxchg64().
> + * Takes u64 parameters.
> + */
> +u64 __cmpxchg_u64(u64 *ptr, u64 old, u64 new)
> +{
> +	raw_spinlock_t *lock = lock_addr(ptr);
> +	unsigned long flags;
> +	u64 prev;
> +
> +	raw_spin_lock_irqsave(lock, flags);
> +	prev = READ_ONCE(*ptr);
> +	if (prev == old)
> +		*ptr = new;
> +	raw_spin_unlock_irqrestore(lock, flags);
> +
> +	return prev;
> +}
> +EXPORT_SYMBOL(__cmpxchg_u64);

This is only going to work if we know that memory modified using
__cmpxchg_u64() is *always* modified using __cmpxchg_u64(). Without that
guarantee there's nothing to stop some other CPU writing to *ptr after
the READ_ONCE() above but before we write new to it.

As far as I'm aware this is not a guarantee we currently provide, so it
would mean making that a requirement for cmpxchg64() users & auditing
them all. That would also leave cmpxchg64() with semantics that differ
from plain cmpxchg(), and semantics that may surprise people. In my view
that's probably not worth it, and it would be better to avoid using
cmpxchg64() on systems that can't properly support it.

For MIPS the problem will go away with the nanoMIPS ISA which includes &
requires LLWP/SCWP (W = word, P = paired) instructions that we can use
to implement cmpxchg64() properly, but of course that won't help older
systems.

Thanks,
    Paul