On Fri, 29 Dec 2023 at 12:56, David Laight <David.Laight@xxxxxxxxxx> wrote:
>
> osq_wait_next() is passed 'prev' from osq_lock() and NULL from osq_unlock()
> but only needs the 'cpu' value to write to lock->tail.
> Just pass prev->cpu or OSQ_UNLOCKED_VAL instead.
>
> Also directly return NULL or 'next' instead of breaking the loop.
Please split these two totally independent things out of the patch,
just to make things much more obvious.
I like the new calling convention, but I don't like how the patch
isn't obviously just that.
In fact, I'd take your patch #1 and just the calling convention change
from #3 as "these are obviously not changing anything at all, only
moving things to more local places".
I'd also take the other part of #3 as a "clearly doesn't change
anything" but it should be a separate patch, and it should be done
differently: make 'next' be local to just *inside* the for-loop (in
fact, make it local to the if-statement that sets it), to clarify the
whole thing that it can never be non-NULL at the top of the loop, and
can never have any long-term semantics.
The other parts actually change some logic, and would need the OSQ
people to take a more serious look.
Linus
