Re: [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 19/10/2023 10:39 am, Jan Beulich wrote:
> On 19.10.2023 11:35, Andrew Cooper wrote:
>> On 19/10/2023 8:48 am, Jan Beulich wrote:
>>> On 13.10.2023 11:42, Juergen Gross wrote:
>>>> Instead of being able to use normal spinlocks as recursive ones, too,
>>>> make recursive spinlocks a special lock type.
>>>>
>>>> This will make the spinlock structure smaller in production builds and
>>>> add type-safety.
>>>>
>>>> This allows to increase the maximum number of physical cpus from 8191
>>>> to 65535 without increasing the size of the lock structure in production
>>>> builds (the size of recursive spinlocks in debug builds will grow to
>>>> 12 bytes due to that change).
>>>>
>>>> Changes in V2:
>>>> - addressed comments by Jan Beulich
>>>> - lots of additional cleanups
>>>> - reorganized complete series
>>>>
>>>> Juergen Gross (13):
>>>>   xen/spinlock: fix coding style issues
>>>>   xen/spinlock: reduce lock profile ifdefs
>>>>   xen/spinlock: make spinlock initializers more readable
>>>>   xen/spinlock: introduce new type for recursive spinlocks
>>>>   xen/spinlock: rename recursive lock functions
>>>>   xen/spinlock: add rspin_[un]lock_irq[save|restore]()
>>>>   xen/spinlock: make struct lock_profile rspinlock_t aware
>>>>   xen/spinlock: add explicit non-recursive locking functions
>>>>   xen/spinlock: add another function level
>>>>   xen/spinlock: add missing rspin_is_locked() and rspin_barrier()
>>>>   xen/spinlock: split recursive spinlocks from normal ones
>>>>   xen/spinlock: remove indirection through macros for spin_*() functions
>>>>   xen/spinlock: support higher number of cpus
>>> Before looking at patches 4 and onwards, I'd like us to settle on the future
>>> of recursive locking in Xen, considering in particular Andrew's objections
>>> to their use in the code base. If the plan was to eliminate them, I'd see
>>> little point in reworking the infrastructure. I'd like to suggest that one
>>> of us tries to remember to put this up as an agenda item for the next
>>> Community Call. But of course the discussion can also happen right here; I
>>> merely expect there might not be much of a reaction.
>> Actually, I consider this series an improvement.  The CPU limit is the
>> most urgent problem to fix.
>>
>> XenServer has just jumped to NR_CPUS=2k in order to support 2024's range
>> of hardware, and it's only going to be a couple of years more before
>> we're stuck given the current spinlocks.
>>
>> I do genuinely think the code and logic would be better without
>> recursive locks, but making that happen is going to be very invasive and
>> complicated.
>>
>> But in the meantime with spinlocks properly separated from recursive
>> locks, it becomes easier IMO to dissuade the introduction of new cases
>> while we unpick the existing ones.
>>
>> And so what if we do end up deleting recursive locks in a few years
>> time?  That's not an argument against doing this untangling now.
> Of course if that was happening only in a few years time, the series
> here is still worthwhile to have. My question was rather towards us
> possibly eliminating recursive locks in the next release cycle.

I find it unlikely that we'd manage to do it in the next release cycle,
even if we did dedicate a significant portion of effort to it.  There
are more urgent things too.

But the same argument works with several years substituted for months. 

~Andrew