Re: [Qemu-devel] [PATCH 2/2] spapr: Adjust default VSMT value for better migration compatibility

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3551 bytes --]

On Mon, Jan 15, 2018 at 10:48:47AM +0100, Greg Kurz wrote:
> On Mon, 15 Jan 2018 18:27:15 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > fa98fbfc "PC: KVM: Support machine option to set VSMT mode" introduced the
> > "vsmt" parameter for the pseries machine type, which controls the spacing
> > of the vcpu ids of thread 0 for each virtual core.  This was done to bring
> > some consistency and stability to how that was done, while still allowing
> > backwards compatibility for migration and otherwise.
> > 
> > The default value we used for vsmt was set to the max of the host's
> > advertised default number of threads and the number of vthreads per vcore
> > in the guest.  This was done to continue running without extra parameters
> > on older KVM versions which don't allow the VSMT value to be changed.
> > 
> > Unfortunately, even that smaller than before leakage of host configuration
> > into guest visible configuration still breaks things.  Specifically a guest
> > with 4 (or less) vthread/vcore will get a different vsmt value when
> > running on a POWER8 (vsmt==8) and POWER9 (vsmt==4) host.  That means the
> > vcpu ids don't line up so you can't migrate between them, though you should
> > be able to.
> > 
> > Long term we really want to make vsmt == smp_threads for sufficiently
> > new machine types.  However, that means that qemu will then require a
> > sufficiently recent KVM (one which supports changing VSMT) - that's still
> > not widely enough deployed to be really comfortable to do.
> > 
> > In the meantime we some default that will work as often as possible.
> 
> s/we some/we need some/ ?

Corrected.

> > This patch changes that default to 8 in all circumstances.  This does
> > change guest visible behaviour (including for existing machine versions)
> > for many cases - just not the most common/important case.
> > 
> > Following is case by case justification for why this is still the least
> > worst option.  Note that any of the old behaviours can still be duplicated
> > after this patch, it's just that it requires manual intervention by
> > setting the vsmt property on the command line.
> > 
> 
> IIUC this unconditionally breaks existing setups that rely on static
> Micro-Threading on a POWER8 host (eg, subcores-per-core=2 on the host
> and smp_threads=4). I have no evidence this is a widely used setup,
> but FWIW it is documented in some IBM RedBooks:

Well.. it will break migration between old and new qemu on the
microthreaded setup,  but fix it between new qemu on microthreaded
setup and new qemu on non-microthreaded setup (old qemu on
microthreaded to old qemu on non-microthreaded was already broken for
the same reasons as p8<->p9).  It's not really obvious to me which is
preferable.

> "Performance Optimization and Tuning Techniques for IBM Power Systems
>  Processors Including IBM POWER8"
> 
> http://www.redbooks.ibm.com/abstracts/sg248171.html?Open
> 
> "IBM PowerKVM: Configuration and Use"
> 
> http://www.redbooks.ibm.com/abstracts/sg248231.html?Open
>
> Maybe the new behaviour could be added for new machine types only ?

I'd really prefer not to.  It makes some existing cases work, but
breaks some other cases.  Given that the old behaviour is inherently
wrong, I'm more inclined to change it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]