Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <53e7e660-9ee0-1177-b34a-365c1397ec3b@intel.com>
Date:   Tue, 13 Dec 2022 10:28:53 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.5.1
Subject: Re: [Intel-gfx] [PATCH 1/5] linux/minmax.h: add non-atomic version of
 xchg
Content-Language: en-US
To:     Arnd Bergmann <arnd@arndb.de>, linux-kernel@vger.kernel.org,
        intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc:     Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
References: <20221209154843.4162814-1-andrzej.hajda@intel.com>
 <b0c38501-440f-42a3-ae0f-dd95527b0e56@app.fastmail.com>
From:   Andrzej Hajda <andrzej.hajda@intel.com>
Organization: Intel Technology Poland sp. z o.o. - ul. Slowackiego 173, 80-298
 Gdansk - KRS 101882 - NIP 957-07-52-316
In-Reply-To: <b0c38501-440f-42a3-ae0f-dd95527b0e56@app.fastmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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