[Intel-gfx] [PATCH 1/2] drm/i915/display: use fetch_and_zero if applicable

[Andrzej Hajda]

On 08.12.2022 16:44, Tvrtko Ursulin wrote:
> 
> On 08/12/2022 15:02, Jani Nikula wrote:
>> On Thu, 08 Dec 2022, "Vivi, Rodrigo" <rodrigo.vivi at intel.com> wrote:
>>> On Thu, 2022-12-08 at 14:32 +0200, Jani Nikula wrote:
>>>> On Thu, 08 Dec 2022, Andrzej Hajda <andrzej.hajda at intel.com> wrote:
>>>>> Simplify the code.
>>>>
>>>> Personally, I absolutely hate fetch_and_zero().
>>>>
>>>> I understand the point, but there are two main traps:
>>>>
>>>> First, the name implies atomicity, which there is none at all.
>>>>
>>>> Second, the name implies it's part of a kernel core header, which it
>>>> isn't, and this just amplifies the first point.
>>>>
>>>> It's surprising and misleading, and those are not things I like about
>>>> interfaces in the kernel.
>>>>
>>>> I would not like to see this proliferate. If fetch_and_zero() was
>>>> atomic
>>>> *and* part of a core kernel header, it would be a different matter.
>>>> But
>>>> I don't think that's going to happen, exactly because it won't be
>>>> atomic
>>>> and the name implies it is.
>>>
>>> +1 here.
>>>
>>> Please let's go the other way around and try to kill macros like this.
>>>
>>> we either kill or we ensure this gets accepted in the core kernel
>>> libraries.
>>
>> Agreed. I'd be fine with either:
>>
>> 1) Get something like this accepted in core kernel headers:
>>
>> #define fetch_and_zero(ptr) xchg(ptr, 0)
>>
>> 2) Do this in i915:
>>
>> @@
>> expression E;
>> @@
>>
>> - fetch_and_zero(E)
>> + xchg(E, 0)
> 
> We don't need atomic so both solution would IMO be bad.

Heh, too late, already sent :)

> 
> We could propose __fetch_and_zero and fetch_and_zero, to mimic 
> __set_bit/set_bit&co for some consistency in terms of atomic vs 
> non-atomic API flavour?
> 

Or non-atomic xchg


> Assuming of course people will think that the long-ish name of the 
> utility macro brings an overall positive cost benefit.
> 
> Worth a try I guess.
> 
> First step I think we need a cocci script for finding the open coded 
> "fetch and zero" pattern. Not my forte but I can try if no one else has 
> an immediate solution or desire to drive the attempt.

About 1600 patterns:
x = y;
y = 0;

but I guess there could be more:
x = xchg(&y, 0);

x = y;
...
y = 0;

custom macros

Regards
Andrzej


> 
> Regards,
> 
> Tvrtko