[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus
From: |
Nikunj A Dadhania |
Subject: |
Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus |
Date: |
Wed, 20 Sep 2017 12:10:48 +0530 |
David Gibson <address@hidden> writes:
> On Wed, Sep 20, 2017 at 10:43:19AM +0530, Nikunj A Dadhania wrote:
>> David Gibson <address@hidden> writes:
>>
>> > On Wed, Sep 20, 2017 at 09:50:24AM +0530, Nikunj A Dadhania wrote:
>> >> David Gibson <address@hidden> writes:
>> >>
>> >> > On Fri, Sep 15, 2017 at 02:39:16PM +0530, Nikunj A Dadhania wrote:
>> >> >> David Gibson <address@hidden> writes:
>> >> >>
>> >> >> > On Fri, Sep 15, 2017 at 01:53:15PM +0530, Nikunj A Dadhania wrote:
>> >> >> >> David Gibson <address@hidden> 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.
>> >> >>
>> >> >> Should I change the global smp_cores here as well ?
>> >> >
>> >> > I'm pretty uneasy with that option.
>> >>
>> >> Me too.
>> >>
>> >> > It would take a fair bit of checking to ensure that changing smp_cores
>> >> > is safe here. An easier to verify option would be to make the generic
>> >> > logic which splits up an unspecified -smp N into cores and sockets
>> >> > more flexible, possibly based on machine options for max values.
>> >> >
>> >> > That might still be more trouble than its worth.
>> >>
>> >> I think the current approach is the simplest and less intrusive, as we
>> >> are handling a case where user has not bothered to provide a detailed
>> >> topology, the best we can do is create single threaded cores equal to
>> >> number of cores.
>> >
>> > No, sorry. Having smp_cores not correspond to the number of cores per
>> > chip in all cases is just not ok. Add an error message if the
>> > topology isn't workable for powernv by all means. But users having to
>> > use a longer command line is better than breaking basic assumptions
>> > about what numbers reflect what topology.
>>
>> Sorry to ask again, as I am still not convinced, we do similar
>> adjustment in spapr where the user did not provide the number of cores,
>> but qemu assumes them as single threaded cores and created
>> cores(boot_cores_nr) that were not same as smp_cores ?
>
> What? boot_cores_nr has absolutely nothing to do with adjusting the
> topology, and it certainly doesn't assume they're single threaded.
When we start a TCG guest and user provides following commandline, e.g.
"-smp 4", smt_threads is set to 1 by default in vl.c. So the guest boots
with 4 cores, each having 1 thread.
Regards
Nikunj
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, (continued)
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, Nikunj A Dadhania, 2017/09/14
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, David Gibson, 2017/09/15
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, Nikunj A Dadhania, 2017/09/15
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, David Gibson, 2017/09/15
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, Nikunj A Dadhania, 2017/09/15
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, David Gibson, 2017/09/19
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, Nikunj A Dadhania, 2017/09/20
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, David Gibson, 2017/09/20
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, Nikunj A Dadhania, 2017/09/20
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, David Gibson, 2017/09/20
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus,
Nikunj A Dadhania <=
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, Nikunj A Dadhania, 2017/09/20
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, David Gibson, 2017/09/20
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, Nikunj A Dadhania, 2017/09/20
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, Cédric Le Goater, 2017/09/20
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, David Gibson, 2017/09/20
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, Nikunj A Dadhania, 2017/09/20
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, Cédric Le Goater, 2017/09/21
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, Igor Mammedov, 2017/09/21
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, David Gibson, 2017/09/22
- Re: [Qemu-devel] [PATCH] ppc/pnv: fix cores per chip for multiple cpus, David Gibson, 2017/09/22