Re: [Intel-gfx] [PATCH 1/5] linux/minmax.h: add non-atomic version of xchg

[Andrzej Hajda <andrzej.hajda@intel.com>]

On 09.12.2022 18:16, Arnd Bergmann wrote:
> On Fri, Dec 9, 2022, at 16:48, Andrzej Hajda wrote:
>> The pattern of setting variable with new value and returning old
>> one is very common in kernel. Usually atomicity of the operation
>> is not required, so xchg seems to be suboptimal and confusing in
>> such cases. Since name xchg is already in use and __xchg is used
>> in architecture code, proposition is to name the macro exchange.
>>
>> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
> 
> While I generally don't like type invariant calling conventions
> of xchg() and cmpxchg(), having a new function that has a similar
> name without being able to tell which one is which from the
> name seems more confusing.
> 
> Since __xchg() is only used on 11 architectures as an internal

Quite big number for 'only' :)

> name for the backing of arch_xchg() or arch_xchg_relaxed(),
> maybe we can instead rename those to __arch_xchg() and use the
> __xchg() name for the new non-atomic version?

I will try, but even compile test will be some challenge, need to find 
cross-compilers for these archs.

Btw exchange is not totally new name, for example C++ uses it [1].

[1]: https://en.cppreference.com/w/cpp/utility/exchange

Regards
Andrzej

> 
>> +/**
>> + * exchange - set variable pointed by @ptr to @val, return old value
>> + * @ptr: pointer to affected variable
>> + * @val: value to be written
>> + *
>> + * This is non-atomic variant of xchg.
>> + */
>> +#define exchange(ptr, val) ({		\
>> +	typeof(ptr) __ptr = ptr;	\
>> +	typeof(*__ptr) __t = *__ptr;	\
> 
> I think you can better express this using __auto_type than typeof(),
> it is now provided by all supported compilers now.
> 
>       Arnd