Re: [PATCHv5 0/2] x86/ept: reduce translation invalidation impact

From: "Tian, Kevin" <kevin.tian@intel.com>
To: David Vrabel <david.vrabel@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: George Dunlap <george.dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Jan Beulich <jbeulich@suse.com>,
	"Nakajima, Jun" <jun.nakajima@intel.com>
Subject: Re: [PATCHv5 0/2] x86/ept: reduce translation invalidation impact
Date: Fri, 18 Dec 2015 07:46:38 +0000	[thread overview]
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D15F74E098@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <1450365426-23167-1-git-send-email-david.vrabel@citrix.com>

> From: David Vrabel [mailto:david.vrabel@citrix.com]
> Sent: Thursday, December 17, 2015 11:17 PM
> 
> This series improves the performance of EPT by further reducing the
> impact of the translation invalidations (ept_sync_domain()). By:
> 
> a) Deferring invalidations until the p2m write lock is released.

This is only for PATCH 2/2 right? No description of PATCH 1/2 in this summary.

> 
> Prior to this change a 16 VCPU guest could not be successfully
> migrated on an (admittedly slow) 160 PCPU box because the p2m write
> lock was held for such extended periods of time.  This starved the
> read lock needed (by the toolstack) to map the domain's memory,
> triggering the watchdog.
> 
> After this change a 64 VCPU guest could be successfully migrated.
> 
> ept_sync_domain() is very expensive because:
> 
> a) it uses on_selected_cpus() and the IPI cost can be particularly
>    high for a multi-socket machine.
> 
> b) on_selected_cpus() is serialized by its own spin lock.
> 
> On this particular box, ept_sync_domain() could take ~3-5 ms.
> 
> Simply using a fair rw lock was not sufficient to resolve this (but it
> was an improvement) as the cost of the ept_sync_domain calls() was
> still delaying the read locks enough for the watchdog to trigger (the
> toolstack maps a batch of 1024 GFNs at a time, which means trying to
> acquire the p2m read lock 1024 times).
> 
> Changes in v5:
> 
> - Fix PoD by explicitly doing an invalidation before reclaiming zero
>   pages.
> - Use the same mechanism for dealing with freeing page table pages.
>   This isn't a common path and its simpler than the deferred list.
> 
> Changes in v4:
> 
> - __ept_sync_domain() is a no-op -- invalidates are done before VMENTER.
> - initialize ept->invalidate to all ones so the initial invalidate is
>   always done.
> 
> Changes in v3:
> 
> - Drop already applied "x86/ept: remove unnecessary sync after
>   resolving misconfigured entries".
> - Replaced "mm: don't free pages until mm locks are released" with
>   "x86/ept: invalidate guest physical mappings on VMENTER".
> 
> Changes in v2:
> 
> - Use a per-p2m (not per-CPU) list for page table pages to be freed.
> - Hold the write lock while updating the synced_mask.
> 
> David