From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [RFC PATCH 2/2] gnttab: refactor locking for better scalability Date: Tue, 12 Nov 2013 13:42:47 +0000 Message-ID: References: <5282007B02000078001023D6@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VgEFO-0000vq-Ee for xen-devel@lists.xenproject.org; Tue, 12 Nov 2013 13:43:14 +0000 Received: by mail-wg0-f43.google.com with SMTP id n12so3043431wgh.34 for ; Tue, 12 Nov 2013 05:43:12 -0800 (PST) In-Reply-To: <5282007B02000078001023D6@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Matt Wilson Cc: Felipe Franciosi , Anthony Liguori , Andrew Cooper , David Vrabel , Matt Wilson , xen-devel@lists.xenproject.org, roger.pau@citrix.com List-Id: xen-devel@lists.xenproject.org On 12/11/2013 09:18, "Jan Beulich" wrote: >>>> On 12.11.13 at 09:07, Keir Fraser wrote: >> On 12/11/2013 07:18, "Matt Wilson" 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 >