[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 5/5] x86/p2m: split write_p2m_entry() hook


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 18 Nov 2020 10:22:30 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=iay9eRW/aq5QUHDL1el8fdyJak5Y00b4sZKLgBI5u/0=; b=dkSG08Sov1HIBOX2/988Zc+tmy9kw2u8HAemsySbKGI/3CQaviiF7mVerHzC0vYzlRaUn4W+by43YTyzaTRz9Ue6fik/AR3UBt+CPQL/QcJxK7dyyd/Qt68uf0E67ZBfrYbujsTOY/INom70I/dfaqQZl7YQFA7UJnKzyz+pPVRgg0oipb9SXFRueiEiy4kLppQDZo1o5pl77Ii6d3/ZRnyVmLM7XTIp7UF8rP6h82p+Dp9jGIO2D1e/XrlqHtOhbfas6dXBHQBY3HnIysuMuuMg5Wq8/wYhGsfiBiL4lDrwMU1kfRZlPJmMTy3/LgowcUieVDS3WHAshAfo+HFVJQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=i5WMXFvwDJTfwBhJvoeL1EnANzj63B43MpIqwIH8vEP84mXNNdw+KOvwXvHmafeAzwK3fQgjrMdC6lmKb9wkSUCG4EK0TWlH98Tum7jVbYJtds+3Xm63B/NJWXHNc739f3YfR8eVGNAypH2ZcvfHW5JFzEkaMwbDfak8mLIYX/U7dmUeJ7ilGlE0JLb3nAYXr0b7jPV1+jwR0HLnsSGvvRYlgWQ+vFWvEPxJEWSprEQkWdNOPeiYa+s/3r041SPArymQ/qB+3W0bs/ZDGjD0iloYPtbUu0Uz2s9i+1/0jBMkyNknPMmYSflcEDxdQCJPydE9gCJpPpFkIx5EIDe3bA==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Tim Deegan <tim@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
  • Delivery-date: Wed, 18 Nov 2020 09:22:47 +0000
  • Ironport-sdr: w7ivJZBB14JlVS6jGGeXexx9ZkbKWZkhg51f+k8lu5WQPs5Ju0M7a7pDn602yfRKR7y2q8SFol y1vSIzYm648bxjgfBehYTOqhDMgNZjYfioSylVJgSYbI15ivdBwuKSQYLQD6Cze1XZyCaW/qQf MbmQJVkSOTTmRO7HYJTvL2XZCUDXvn/cN706vNLSjy5VZ9vdvvsDKPsgW8cD4W6zu7GDq1W0Uc XoEVzzF1w7ZLOvD+HRt/7GB55LNX+WsQK138t2Y4O/XPnWhQRU235/e67LmhHmKIF7tv1UR1rz 8VQ=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Fri, Nov 13, 2020 at 10:52:58AM +0100, Jan Beulich wrote:
> On 12.11.2020 18:52, Tim Deegan wrote:
> > At 15:04 +0100 on 12 Nov (1605193496), Jan Beulich wrote:
> >> On 12.11.2020 14:07, Roger Pau Monné wrote:
> >>> On Thu, Nov 12, 2020 at 01:29:33PM +0100, Jan Beulich wrote:
> >>>> I agree with all this. If only it was merely about TLB flushes. In
> >>>> the shadow case, shadow_blow_all_tables() gets invoked, and that
> >>>> one - looking at the other call sites - wants the paging lock held.
> > [...]
> >>> The post hook for shadow could pick the lock again, as I don't think
> >>> the removal of the tables needs to be strictly done inside of the same
> >>> locked region?
> >>
> >> I think it does, or else a use of the now stale tables may occur
> >> before they got blown away. Tim?
> > 
> > Is this the call to shadow_blow_tables() in the write_p2m_entry path?
> 
> Yes.
> 
> > I think it would be safe to drop and re-take the paging lock there as
> > long as the call happens before the write is considered to have
> > finished.
> > 
> > But it would not be a useful performance improvement - any update that
> > takes this path is going to be very slow regardless.  So unless you
> > have another pressing reason to split it up, I would be inclined to
> > leave it as it is.  That way it's easier to see that the locking is
> > correct.
> 
> Thanks for the clarification.
> 
> Roger - what's your view at this point?

So my main concern was not really about making this path faster -
after all this shouldn't be a hot path anyway, but rather reducing the
region protected by the paging lock, since it's a global lock that's
quite contended. In the HAP case we could move the flush outside of
the locked region thus reducing the lock time.

Anyway, seeing there's not much consensus on this aspect leaving it
as-is is no worse that what's currently in there.

Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.