Re: [RFC PATCH 0/3] xen/spinlock: make recursive spinlocks a dedicated type

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.12.2022 17:36, Juergen Gross wrote:
> On 14.12.22 17:25, Daniel P. Smith wrote:
>> On 12/14/22 10:31, Juergen Gross wrote:
>>> On 14.12.22 16:03, Daniel P. Smith wrote:
>>>>
>>>> On 9/10/22 11:49, 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.
>>>>
>>>> Just a little yak shaving, IMHO a spinlock is normally not expected to be 
>>>> recursive. Thus explicitly naming a spinlock as non-recursive I find to be 
>>>> redundant along with being expensive for typing. Whereas a recursive 
>>>> spinlock 
>>>> is the special instance and should have a declarative distinction. Only 
>>>> codifying the recursive type would significantly cut down on the size of 
>>>> the 
>>>> series and still provide equal type and visual clarification.
>>>
>>> A "normal" spinlock is non-recursive.
>>>
>>> A recursive spinlock in Xen can be either taken recursive, or it can be 
>>> taken
>>> non-recursive, causing further recursive attempts to spin.
>>
>> Yes, I understand the current situation.
>>
>>> So the explicit non-recursive locking applies to that special treatment of
>>> recursive spinlocks.
>>
>> I understand this, but to help clarify what I am saying is that individuals 
>> coming from outside Xen would expect is the spinlock family of calls to 
>> behave 
>> as a non-recursive spinlocks and recursive spinlock family of calls would 
>> provide the recursive behavior. Currently Xen's behavior is backwards to 
>> this, 
>> which this series continues and is a valid approach. Here spinlock and 
>> recursive 
>> spinlock family of calls are recursive and must use non-recursive spinlock 
>> family to have "normal" spinlock behavior. IMHO it would greatly simplify 
>> the 
> 
> My series doesn't change treatment of normal spinlocks. They are still used 
> via
> spin_{lock,locked,unlock}.
> 
>> code and align with the "normal" understanding of spinlocks if instead 
>> spin_{lock,locked,unlock} macros were the non-recursive calls and 
>> spin_{lock,locked,unlock}_recursive macros were the recursive calls. Then 
>> there 
>> would only be two suites of calls for spinlocks and a lot less keystrokes 
>> for 
>> need for most development.
> 
> Only the recursive spinlock type user must specify, whether a lock is meant to
> be handled as a recursive or a non-recursive lock attempt. This is similar to
> a rwlock, where the user must specify whether to lock as a reader or a writer.

While I can't come up with anything neat right away, it feels like it should be
possible to come up with some trickery to make spin_lock() usable on both lock
types, eliminating the need to ..._nonrecursive() variants visible at use sites
(they may still be necessary as helpers then). At least if a spinlock_t instance
wasn't embedded in the recursive lock struct (as I did suggest), then something
along the lines of what tgmath.h does may be possible.

Jan