Handle cas failure when the compare succeeds

[andrew]

Summary

When locking a priority inherit mutex we perform a compare and swap
operation to try and acquire the mutex. This may fail even when the
compare succeeds.

Check and handle this case.

PR: 263825

[kib]

I checked for the described condition by testing for rv == 1. Then it does not need this long comment.

sys/kern/kern_umtx.c

2269

[andrew]

Trim the comment

[andrew]

rv == 1 could be because the compare failed, or the store failed. The check is for the store case that was missing.

[dchagin]

sys/kern/kern_umtx.c

2269

May be put this under #if defined (arm)? Do we have some common define for ll/sc?

[markj]

sys/kern/kern_umtx.c

2269

I don't believe we have any such define. It is not really possible to write one, I think: arm64 has two sets of atomic RMW operations, one using LL/SC (required) and one using atomic instructions (part of the LSE extension, not required in all ARMv8 implementations). We don't know at compile-time which flavour will be used; the kernel will look at the CPU capabilities and use LSE if available.

[markj]

sys/kern/kern_umtx.c

2269

Oops, the last sentence is wrong, it applies only to atomic(9). Currently casueword32() is always implemented using LL/SC atomics. But I think my argument still holds. We should possibly provide LSE-based implementations of casueword32() etc., since some ARMv8 implementations (graviton 2 in particular) have significantly better performance with LSE.

Is there any problem if userspace uses LL/SC atomics and the kernel uses LSE atomics when both are modifying the same word? I'd hope not...

[dchagin]

sys/kern/kern_umtx.c

2269

Thank you for your comprehensive answer.
The same fix is needed for linux_futex_lock_pi as it is modeled after umtx. I'm plan to ifdef it under aarch64.

[markj]

sys/kern/kern_umtx.c

2269

Why the ifdef? What about other LL/SC platforms, like riscv? IMHO it is fine to simply check unconditionally.

[andrew]

sys/kern/kern_umtx.c

2269

If we think this is a problem we could add a macro for architectures where the casu functions may use ll/sc atomics.