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: 2253 bytes --]

On Thu, Sep 14, 2017 at 10:42:52AM +0530, Nikunj A Dadhania wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > On Mon, Sep 11, 2017 at 10:40:10AM +0530, Nikunj A Dadhania wrote:
> >> David Gibson <david@gibson.dropbear.id.au> writes:
> >> 
> >> > On Wed, Sep 06, 2017 at 01:57:48PM +0530, Nikunj A Dadhania wrote:
> >> >> When the user does not provide the cpu topology, e.g. "-smp 4", machine fails to
> >> >> initialize 4 cpus. Compute the chip per cores depending on the number of chips
> >> >> and smt threads.
> >> >> 
> >> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >> >
> >> > I don't understand why simply treating smp_cores as cores per chip is wrong.
> >> 
> >> We do not have SMT support and when "-smp 4" is passed, smp_cores is
> >> always set to 1. So only once core with one thread finally show up in
> >> the guest. Moreover, I see spapr too doing similar thing in
> >> spapr_init_cpus() with boot_cores_nr.
> >
> > I'm ok with adding an error if smp_threads > 1, since that won't
> > work.  Breaking the identity that smp_cores is the number of cores per
> > socket (which should correspond to one chip) is not ok, though.
> >
> > I think you're misinterpreting the boot_cores_nr stuff - that's just
> > about translating the number of initially online vcpus into a number
> > of initially online cores.
> 
> 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.

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