On 08/12/2022 15:02, Jani Nikula wrote:
On Thu, 08 Dec 2022, "Vivi, Rodrigo" <rodrigo.vivi@xxxxxxxxx> wrote:
On Thu, 2022-12-08 at 14:32 +0200, Jani Nikula wrote:
On Thu, 08 Dec 2022, Andrzej Hajda <andrzej.hajda@xxxxxxxxx> 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
