Re: [Xen-devel] [RFC PATCH 2/2] gnttab: refactor locking for better scalability

[Keir Fraser <keir.xen@xxxxxxxxx>]

On 12/11/2013 09:18, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:

>>>> On 12.11.13 at 09:07, Keir Fraser <keir.xen@xxxxxxxxx> wrote:
>> On 12/11/2013 07:18, "Matt Wilson" <msw@xxxxxxxxx> wrote:
>> 
>>> Yes, I'm a little concerned about writer starvation. But so far even
>>> in the presence of very frequent readers it seems like the infrequent
>>> writers are able to get the lock when they need to. However, I've not
>>> tested the iommu=strict path yet. I'm thinking that in that case
>>> there's just going to be frequent writers, so there's less risk of
>>> readers starving writers. For what it's worth, when mapcount() gets in
>>> the picture with persistent grants, I'd expect to see some pretty
>>> significant performance degradation for map/unmap operations. This was
>>> also observed in [1] under different circumstances.
>> 
>> The average case isn't the only concern here, but also the worst case, which
>> could maybe tie up a CPU for unbounded time. Could a malicious guest set up
>> such a workload? I'm just thinking we don't want to end up with a DoS XSA on
>> this down the line. :)
> 
> And indeed I think we should be making our rwlocks fair for writers
> before pushing in the change here; I've been meaning to get to this
> for a while, but other stuff continues to require attention. I'm also
> of the opinion that we should switch to ticket spinlocks.

Would queuing spinlocks (e.g. MCS locks) be even more preferable? Two atomic
ops (cmpxchg) per critical region in the uncontended case. Each CPU spins on
its own location so there's no cacheline carnage in the highly contended
case (a problem with simple ticket spinlocks). And it builds on cmpxchg so
the spinlock implementation has no arch-specific component (apart from
cmpxchg, which we already have).

I have a queue-based rwlock design too, does require a spinlock lock/unlock
per rwlock op though (i.e., 4 atomic ops per critical region in the
uncontended case).

> But of course, fairness for writers means that performance may
> drop again on the read paths, unless the write lock use is strictly
> limited to code paths not (normally) involved in I/O.

Yes indeed. Hence depending on other users of rwlock, just letting a writer
set its flag and wait for in-flight readers to drain might be sufficient for
now.

 -- Keir

> Jan
> 



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