Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus

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

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

On Fri, Sep 15, 2017 at 01:53:15PM +0530, Nikunj A Dadhania wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> >> 
> >> I thought, I am doing the same here for PowerNV, number of online cores
> >> is equal to initial online vcpus / threads per core
> >> 
> >>    int boot_cores_nr = smp_cpus / smp_threads;
> >> 
> >> Only difference that I see in PowerNV is that we have multiple chips
> >> (max 2, at the moment)
> >> 
> >>         cores_per_chip = smp_cpus / (smp_threads * pnv->num_chips);
> >
> > This doesn't make sense to me.  Cores per chip should *always* equal
> > smp_cores, you shouldn't need another calculation for it.
> >
> >> And in case user has provided sane smp_cores, we use it.
> >
> > If smp_cores isn't sane, you should simply reject it, not try to fix
> > it.  That's just asking for confusion.
> 
> This is the case where the user does not provide a topology(which is a
> valid scenario), not sure we should reject it. So qemu defaults
> smp_cores/smt_threads to 1. I think it makes sense to over-ride.

If you can find a way to override it by altering smp_cores when it's
not explicitly specified, then ok.

But overriding smp_cores with a different variable that's the "real"
number of cores is not acceptable.  If that means the user has to
specify cores explicitly, so be it.  Slight awkwardness in command
line is preferable to breaking the assumption that smp_cores == (# of
cores per next level up cpu object) which is used all over the place.

-- 
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 --]