Re: [Xen-devel] [PATCH] x86/domain_page: implement pure per-vCPU mapping infrastructure

["Xia, Hongyan" <hongyxia@xxxxxxxxxx>]

On Tue, 2020-02-04 at 12:08 +0000, Wei Liu wrote:
> Thanks, I welcome effort to make Xen more scalable. :-) 
> 
> On Mon, Feb 03, 2020 at 06:36:53PM +0000, Hongyan Xia wrote:
> > Rewrite the mapcache to be purely per-vCPU instead of partly per-
> > vCPU
> > and partly per-domain.
> > 
> > This patch is needed to address performance issues when we start
> > relying
> > on the mapcache, e.g., when we do not have a direct map. Currently,
> > the
> > per-domain lock on the mapcache is a bottleneck for multicore,
> > causing
> > performance degradation and even functional regressions. This patch
> > makes the mapping structure per-vCPU and completely lockless.
> > 
> 
> When I see "regression", I think of something that was working before
> but not anymore. I don't think that's the case for the following two
> things. 
> 
> I would just classify them as bug and/or improvement.

We probably have different definitions for "regression"... but I can
reword.

> > Functional regression:
> > 
> > When a domain is run on more than 64 cores, FreeBSD 10 panicks
> > frequently
> > due to occasional simultaneous set_singleshot_timer hypercalls from
> > too
> > many cores. Some cores will be blocked waiting on map_domain_page,
> > eventually failing to set a timer in the future. FreeBSD cannot
> > handle
> > this and panicks. This was fixed in later FreeBSD releases by
> > handling
> > -ETIME, but still the degradation in timer performance is a big
> > issue.
> > 
> > Performance regression:
> > 
> > Many benchmarks see a performance drop when having a large core
> > count.
> > I have done a Geekbench on a 32-vCPU guest.
> > 
> > perf drop     old        new
> > single       0.04%      0.18%
> > multi        2.43%      0.08%
> > 
> > Removing the per-domain lock in the mapcache brings the multi-core
> > performance almost identical to using the direct map for mappings.
> > 
> > There should be room for futher optimisations, but this already
> > improves over the old mapcache by a lot.
> > 
> > Note that entries in the maphash will occupy inuse slots. With 16
> > slots
> > per vCPU and a maphash capacity of 8, we only have another 8
> > available,
> > which is not enough for nested page table walks. We need to
> > increase the
> > number of slots in config.h.
> > 
> > Signed-off-by: Hongyan Xia <hongyxia@xxxxxxxxxx>
> 
> 
> As far as I can tell all vcpus still share the same per-domain
> mapcache
> region. The difference is now the region is divided into subregion
> for
> each vcpu to use.

You are right. We have a per-domain VA range and we have
create_perdomain_mappings which can be reused nicely. The patch divides
the regions into vCPUs.

> 
> > ---
> >  xen/arch/x86/domain.c        |   5 +-
> >  xen/arch/x86/domain_page.c   | 229 +++++++++++------------------
> > ------
> >  xen/include/asm-x86/config.h |   2 +-
> >  xen/include/asm-x86/domain.h |  30 +----
> >  4 files changed, 80 insertions(+), 186 deletions(-)
> > 
> > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> > index f53ae5ff86..a278aa4678 100644
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -445,6 +445,9 @@ void arch_vcpu_destroy(struct vcpu *v)
> >      xfree(v->arch.msrs);
> >      v->arch.msrs = NULL;
> >  
> > +    xfree(v->arch.pv.mapcache);
> > +    v->arch.pv.mapcache = NULL;
> > +
> 
> Keep in mind that accessing the union {pv, hvm} before knowing the
> exact variant is dangerous.
> 
> Because mapcache is initialised for PV only, so it should be freed
> for
> PV only. I think you need to put it to pv_vcpu_destroy.
> 
> If you don't want to do that because of asymmetry with
> mapcache_vcpu_init, you may want to invent mapcache_vcpu_destroy for
> your purpose.

Ah right this is a problem. I was working on a tree where everyone has
a mapcache, which is not true for current upstream when I cherry-
picked. Will fix.

Hongyan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel