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

[Tvrtko Ursulin]

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.

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?

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.

Regards,

Tvrtko