On Mon, Jul 11, 2022 at 05:33:01PM +0200, Uros Bizjak wrote:
> Use try_cmpxchg family of functions instead of cmpxchg (*ptr, old, new) == old.
> x86 CMPXCHG instruction returns success in ZF flag, so this change saves a
> compare after cmpxchg (and related move instruction in front of cmpxchg).
>
> Also, try_cmpxchg implicitly assigns old *ptr value to "old" when
> cmpxchg fails, enabling further code simplifications.
>
> No functional change intended.
You might want to split this into a patch per caller as it might
attact different reviewers.
> + do {
> + } while (old && !atomic_try_cmpxchg(&blkg->use_delay, &old, old - 1));
It might just be me, but for loops with an empty body this do { } while
construct looks odd. Why not:
while (old && !atomic_try_cmpxchg(&blkg->use_delay, &old, old - 1))
;
?
The the use of the atomic on ->use_delay looks really whacky to start
with. To me it sounds like it really wants to use a proper lock
instead of all this magic.
> else
> return;
>
> - old = atomic_cmpxchg(&iolat->scale_cookie, our_cookie, cur_cookie);
> -
> - /* Somebody beat us to the punch, just bail. */
> - if (old != our_cookie)
> + if (!atomic_try_cmpxchg(&iolat->scale_cookie, &our_cookie, cur_cookie)) {
> + /* Somebody beat us to the punch, just bail. */
> return;
> + }
/* If somebody beat us to the punch, just bail. */
if (!atomic_try_cmpxchg(&iolat->scale_cookie, &our_cookie, cur_cookie))
return;
