Re: [Xen-devel] [PATCH 1/2] rwlock: add per-cpu reader-writer locks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.11.15 at 17:21, <malcolm.crossley@xxxxxxxxxx> wrote:
> I have a suggestion to remove the "no accessing two percpu rwlock resources
> simultaneously on the same PCPU" restriction:
> 
> On accessing the second resource, we can detect that the percpu read area is 
> already
> in use, if this is the case then we can just take standard read_lock on the 
> second
> resource instead of using the percpu read area. This makes the percpu rw 
> lock safe
> to use on multiple resources simulatenously but it falls back to standard rw 
> lock
> performance for taking a two or more percpu rw lock simultaneously.

Yes, that's an option.

> I'm suggesting the above option instead of per resource (NR_DOM_IDS), percpu 
> data areas
> because there may be cases where the number of resources is very large and 
> so
> difficult/expensive to preallocate as percpu data areas.

Well, typically (grant lock, p2m lock) these would be per domain, and
with the 32k limit on domains this wouldn't be too bad (but also not
too nice, I agree).

>>>>> +    cpumask_t active_readers;
>>>>
>>>> Did you consider ways to avoid such a large variable to be put on
>>>> the stack, and none turned out suitable?
>>>
>>> I wasn't aware the cpumask_t variable at 32 bytes was considered problematic
>>> for the stack. I can't think of any way around the issue without causing
>>> further restrictions (a percpu area will prevent taking two different percpu
>>> rwlocks at the same time).
>> 
>> 32 bytes it is only for 256 CPU builds. What about a 4095 CPU config?
>> 
>> Wouldn't it be possible (either excluding use of such locks from
>> interrupt context, or by disabling interrupts for the duration of
>> the mask's use) to have a per-CPU mask?
> 
> That sounds possible, I'd prefer preventing the use of the lock in
> irq context so that the common case does have the penalty of disabling IRQS.

Which seems reasonable, and documentable by not having _irq
and _irqsave/_irqrestore flavors.

I have to admit that I didn't really follow the rest of your
argumentation here, since the moment we don't allow these locks
in interrupts, all problems should be gone.

>>> What requirements would there be for a generic percpu rwlock implementation
>>> to be accepted?
>> 
>> At least the multiple resource lock issue should be avoided, and I think
>> I've pointed out before how I think this can be achieved without any
>> fundamental changes.
> 
> Does the suggestion above satisfy this requirement?

Yes, even if I like the other alternative better.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel