Re: [Xen-devel] [PATCH v3 1/5] xen/spinlocks: in debug builds store cpu holding the lock

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.09.2019 16:31, Juergen Gross wrote:
> On 03.09.19 16:11, Jan Beulich wrote:
>> On 29.08.2019 12:18, Juergen Gross wrote:
>>> --- a/xen/include/xen/spinlock.h
>>> +++ b/xen/include/xen/spinlock.h
>>> @@ -6,14 +6,21 @@
>>>   #include <asm/types.h>
>>>   
>>>   #ifndef NDEBUG
>>> -struct lock_debug {
>>> -    s16 irq_safe; /* +1: IRQ-safe; 0: not IRQ-safe; -1: don't know yet */
>>> +union lock_debug {
>>> +    uint16_t val;
>>> +#define LOCK_DEBUG_INITVAL 0xffff
>>> +    struct {
>>> +        uint16_t cpu:12;
>>
>> I'm afraid I have one more request: The literal 12 in struct spinlock is
>> sitting right next to the SPINLOCK_NO_CPU definition, making it pretty
>> unlikely that both author and reviewer of a change wouldn't notice one
>> getting changed without adjustment to the other. The literal 12 here,
>> though, is sufficiently remote to that other place, so there needs to be
>> a connection established. Best I can think of right away is to have a
>> #define for the 12, move SPINLOCK_NO_CPU's definition next to it, and
>> use the new constant in both places.
> 
> Can do.
> 
>>
>>> +        uint16_t pad:2;
>>
>> While at it, would you mind making this an unnamed field?
> 
> NP. I guess the "2" should then be replaced to depend on the added
> #define above.

Ah yes, I didn't even notice this yet.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel