On 03.11.2025 12:26, Andrew Cooper wrote: > On 03/11/2025 10:11 am, Dmytro Prokopchuk1 wrote: >> diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c >> index 0389293b09..d9dc9998e6 100644 >> --- a/xen/common/spinlock.c >> +++ b/xen/common/spinlock.c >> @@ -367,7 +367,8 @@ static void always_inline >> spin_unlock_common(spinlock_tickets_t *t, >> LOCK_PROFILE_REL; >> rel_lock(debug); >> arch_lock_release_barrier(); >> - add_sized(&t->head, 1); >> + BUILD_BUG_ON(sizeof(t->head) != sizeof(uint16_t)); >> + add_u16_sized(&t->head, 1); > > This is an example where MISRA's opinions actively making the logic less > safe. > > It's not possible for add_sized() to use the wrong type (as it > calculates it internally), whereas it's quite possible to update the > BUILD_BUG_ON() and fail to adjust the add. > > Specifically, you've made it more complicated to reason about, and > created an opportunity to make an unsafe change where that opportunity > does not exist in the code as-is. > > Furthermore, read and write atomic have exactly the same internal > pattern as add_sized(), so how does this not just kick the can down the > road?
+1 (fwiw) Jan
