| Message ID | 20210719032043.25416-1-wangyanan55@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | machine: smp parsing fixes and improvement | expand |
On Mon, Jul 19 2021, Yanan Wang <wangyanan55@huawei.com> wrote: > Hi, > > This is v2 of the series [1] that I have posted to introduce some smp parsing > fixes and improvement, much more work has been processed compared to RFC v1. > > [1] https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg00259.html > > The purpose of this series is to improve/fix the parsing logic. Explicitly > specifying a CPU topology parameter as zero is not allowed any more, and > maxcpus is now uniformly used to calculate the omitted parameters. It's also > suggested that we should start to prefer cores over sockets over threads on > the newer machine types, which will make the computed virtual topology more > reflective of the real hardware. > > In order to reduce code duplication and ease the code maintenance, smp_parse > in now converted into a parser generic enough for all arches, so that the PC > specific one can be removed. It's also convenient to introduce more topology > members (e.g. cluster) to the generic parser in the future. Cc:ing Pierre, as he also had been looking at the smp parsing code (for s390x) recently. Also, please keep me on cc: for patches that touch s390x. > > Finally, a QEMU unit test for the parsing of given SMP configuration is added. > Since all the parsing logic is in generic function smp_parse(), this test > passes diffenent SMP configurations to the function and compare the parsing > result with what is expected. In the test, all possible collections of the > topology parameters and the corressponding expected results are listed, > including the valid and invalid ones. The preference of sockets over cores > and the preference of cores over sockets, and the support of multi-dies are > also taken into consideration. > > --- > > Changelogs: > > v1->v2: > - disallow "anything=0" in the smp configuration (Andrew) > - make function smp_parse() a generic helper for all arches > - improve the error reporting in the parser > - start to prefer cores over sockets since 6.2 (Daniel) > - add a unit test for the smp parsing (Daniel) > > --- > > Yanan Wang (11): > machine: Disallow specifying topology parameters as zero > machine: Make smp_parse generic enough for all arches > machine: Uniformly use maxcpus to calculate the omitted parameters > machine: Use the computed parameters to calculate omitted cpus > machine: Improve the error reporting of smp parsing > hw: Add compat machines for 6.2 > machine: Prefer cores over sockets in smp parsing since 6.2 > machine: Use ms instead of global current_machine in sanity-check > machine: Tweak the order of topology members in struct CpuTopology > machine: Split out the smp parsing code > tests/unit: Add a unit test for smp parsing > > MAINTAINERS | 2 + > hw/arm/virt.c | 10 +- > hw/core/machine-smp.c | 124 ++++ > hw/core/machine.c | 68 +-- > hw/core/meson.build | 1 + > hw/i386/pc.c | 66 +-- > hw/i386/pc_piix.c | 15 +- > hw/i386/pc_q35.c | 14 +- > hw/ppc/spapr.c | 16 +- > hw/s390x/s390-virtio-ccw.c | 15 +- > include/hw/boards.h | 13 +- > include/hw/i386/pc.h | 3 + > qapi/machine.json | 6 +- > qemu-options.hx | 4 +- > tests/unit/meson.build | 1 + > tests/unit/test-smp-parse.c | 1117 +++++++++++++++++++++++++++++++++++ > 16 files changed, 1338 insertions(+), 137 deletions(-) > create mode 100644 hw/core/machine-smp.c > create mode 100644 tests/unit/test-smp-parse.c > > -- > 2.19.1
On 2021/7/20 0:57, Cornelia Huck wrote: > On Mon, Jul 19 2021, Yanan Wang <wangyanan55@huawei.com> wrote: > >> Hi, >> >> This is v2 of the series [1] that I have posted to introduce some smp parsing >> fixes and improvement, much more work has been processed compared to RFC v1. >> >> [1] https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg00259.html >> >> The purpose of this series is to improve/fix the parsing logic. Explicitly >> specifying a CPU topology parameter as zero is not allowed any more, and >> maxcpus is now uniformly used to calculate the omitted parameters. It's also >> suggested that we should start to prefer cores over sockets over threads on >> the newer machine types, which will make the computed virtual topology more >> reflective of the real hardware. >> >> In order to reduce code duplication and ease the code maintenance, smp_parse >> in now converted into a parser generic enough for all arches, so that the PC >> specific one can be removed. It's also convenient to introduce more topology >> members (e.g. cluster) to the generic parser in the future. > Cc:ing Pierre, as he also had been looking at the smp parsing code (for > s390x) recently. > > Also, please keep me on cc: for patches that touch s390x. Sure, I will. Sorry about the missing. :) Thanks, Yanan . >> Finally, a QEMU unit test for the parsing of given SMP configuration is added. >> Since all the parsing logic is in generic function smp_parse(), this test >> passes diffenent SMP configurations to the function and compare the parsing >> result with what is expected. In the test, all possible collections of the >> topology parameters and the corressponding expected results are listed, >> including the valid and invalid ones. The preference of sockets over cores >> and the preference of cores over sockets, and the support of multi-dies are >> also taken into consideration. >> >> --- >> >> Changelogs: >> >> v1->v2: >> - disallow "anything=0" in the smp configuration (Andrew) >> - make function smp_parse() a generic helper for all arches >> - improve the error reporting in the parser >> - start to prefer cores over sockets since 6.2 (Daniel) >> - add a unit test for the smp parsing (Daniel) >> >> --- >> >> Yanan Wang (11): >> machine: Disallow specifying topology parameters as zero >> machine: Make smp_parse generic enough for all arches >> machine: Uniformly use maxcpus to calculate the omitted parameters >> machine: Use the computed parameters to calculate omitted cpus >> machine: Improve the error reporting of smp parsing >> hw: Add compat machines for 6.2 >> machine: Prefer cores over sockets in smp parsing since 6.2 >> machine: Use ms instead of global current_machine in sanity-check >> machine: Tweak the order of topology members in struct CpuTopology >> machine: Split out the smp parsing code >> tests/unit: Add a unit test for smp parsing >> >> MAINTAINERS | 2 + >> hw/arm/virt.c | 10 +- >> hw/core/machine-smp.c | 124 ++++ >> hw/core/machine.c | 68 +-- >> hw/core/meson.build | 1 + >> hw/i386/pc.c | 66 +-- >> hw/i386/pc_piix.c | 15 +- >> hw/i386/pc_q35.c | 14 +- >> hw/ppc/spapr.c | 16 +- >> hw/s390x/s390-virtio-ccw.c | 15 +- >> include/hw/boards.h | 13 +- >> include/hw/i386/pc.h | 3 + >> qapi/machine.json | 6 +- >> qemu-options.hx | 4 +- >> tests/unit/meson.build | 1 + >> tests/unit/test-smp-parse.c | 1117 +++++++++++++++++++++++++++++++++++ >> 16 files changed, 1338 insertions(+), 137 deletions(-) >> create mode 100644 hw/core/machine-smp.c >> create mode 100644 tests/unit/test-smp-parse.c >> >> -- >> 2.19.1 > .
> > On Mon, Jul 19 2021, Yanan Wang <wangyanan55@huawei.com> wrote: > > > >> Hi, > >> > >> This is v2 of the series [1] that I have posted to introduce some smp parsing > >> fixes and improvement, much more work has been processed compared to RFC v1. > >> > >> [1] https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg00259.html > >> > >> The purpose of this series is to improve/fix the parsing logic. Explicitly > >> specifying a CPU topology parameter as zero is not allowed any more, and > >> maxcpus is now uniformly used to calculate the omitted parameters. It's also > >> suggested that we should start to prefer cores over sockets over threads on > >> the newer machine types, which will make the computed virtual topology more > >> reflective of the real hardware. > >> > >> In order to reduce code duplication and ease the code maintenance, smp_parse > >> in now converted into a parser generic enough for all arches, so that the PC > >> specific one can be removed. It's also convenient to introduce more topology > >> members (e.g. cluster) to the generic parser in the future. > > Cc:ing Pierre, as he also had been looking at the smp parsing code (for > > s390x) recently. > > > > Also, please keep me on cc: for patches that touch s390x. > Sure, I will. Sorry about the missing. :) > > Thanks, > Yanan > . > >> Finally, a QEMU unit test for the parsing of given SMP configuration is added. > >> Since all the parsing logic is in generic function smp_parse(), this test > >> passes diffenent SMP configurations to the function and compare the parsing > >> result with what is expected. In the test, all possible collections of the > >> topology parameters and the corressponding expected results are listed, > >> including the valid and invalid ones. The preference of sockets over cores > >> and the preference of cores over sockets, and the support of multi-dies are > >> also taken into consideration. > >> > >> --- > >> > >> Changelogs: > >> > >> v1->v2: > >> - disallow "anything=0" in the smp configuration (Andrew) > >> - make function smp_parse() a generic helper for all arches > >> - improve the error reporting in the parser > >> - start to prefer cores over sockets since 6.2 (Daniel) > >> - add a unit test for the smp parsing (Daniel) > >> > >> --- > >> > >> Yanan Wang (11): > >> machine: Disallow specifying topology parameters as zero > >> machine: Make smp_parse generic enough for all arches > >> machine: Uniformly use maxcpus to calculate the omitted parameters > >> machine: Use the computed parameters to calculate omitted cpus > >> machine: Improve the error reporting of smp parsing > >> hw: Add compat machines for 6.2 > >> machine: Prefer cores over sockets in smp parsing since 6.2 > >> machine: Use ms instead of global current_machine in sanity-check > >> machine: Tweak the order of topology members in struct CpuTopology > >> machine: Split out the smp parsing code > >> tests/unit: Add a unit test for smp parsing > >> > >> MAINTAINERS | 2 + > >> hw/arm/virt.c | 10 +- > >> hw/core/machine-smp.c | 124 ++++ > >> hw/core/machine.c | 68 +-- > >> hw/core/meson.build | 1 + > >> hw/i386/pc.c | 66 +-- > >> hw/i386/pc_piix.c | 15 +- > >> hw/i386/pc_q35.c | 14 +- > >> hw/ppc/spapr.c | 16 +- > >> hw/s390x/s390-virtio-ccw.c | 15 +- > >> include/hw/boards.h | 13 +- > >> include/hw/i386/pc.h | 3 + > >> qapi/machine.json | 6 +- > >> qemu-options.hx | 4 +- > >> tests/unit/meson.build | 1 + > >> tests/unit/test-smp-parse.c | 1117 +++++++++++++++++++++++++++++++++++ > >> 16 files changed, 1338 insertions(+), 137 deletions(-) > >> create mode 100644 hw/core/machine-smp.c > >> create mode 100644 tests/unit/test-smp-parse.c > >> Please add me in CC as I reviewed one of the patch and was in middle of reviewing few other patches but missed the latest version. Thanks, Pankaj
On 2021/7/21 21:52, Pankaj Gupta wrote: >>> On Mon, Jul 19 2021, Yanan Wang <wangyanan55@huawei.com> wrote: >>> >>>> Hi, >>>> >>>> This is v2 of the series [1] that I have posted to introduce some smp parsing >>>> fixes and improvement, much more work has been processed compared to RFC v1. >>>> >>>> [1] https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg00259.html >>>> >>>> The purpose of this series is to improve/fix the parsing logic. Explicitly >>>> specifying a CPU topology parameter as zero is not allowed any more, and >>>> maxcpus is now uniformly used to calculate the omitted parameters. It's also >>>> suggested that we should start to prefer cores over sockets over threads on >>>> the newer machine types, which will make the computed virtual topology more >>>> reflective of the real hardware. >>>> >>>> In order to reduce code duplication and ease the code maintenance, smp_parse >>>> in now converted into a parser generic enough for all arches, so that the PC >>>> specific one can be removed. It's also convenient to introduce more topology >>>> members (e.g. cluster) to the generic parser in the future. >>> Cc:ing Pierre, as he also had been looking at the smp parsing code (for >>> s390x) recently. >>> >>> Also, please keep me on cc: for patches that touch s390x. >> Sure, I will. Sorry about the missing. :) >> >> Thanks, >> Yanan >> . >>>> Finally, a QEMU unit test for the parsing of given SMP configuration is added. >>>> Since all the parsing logic is in generic function smp_parse(), this test >>>> passes diffenent SMP configurations to the function and compare the parsing >>>> result with what is expected. In the test, all possible collections of the >>>> topology parameters and the corressponding expected results are listed, >>>> including the valid and invalid ones. The preference of sockets over cores >>>> and the preference of cores over sockets, and the support of multi-dies are >>>> also taken into consideration. >>>> >>>> --- >>>> >>>> Changelogs: >>>> >>>> v1->v2: >>>> - disallow "anything=0" in the smp configuration (Andrew) >>>> - make function smp_parse() a generic helper for all arches >>>> - improve the error reporting in the parser >>>> - start to prefer cores over sockets since 6.2 (Daniel) >>>> - add a unit test for the smp parsing (Daniel) >>>> >>>> --- >>>> >>>> Yanan Wang (11): >>>> machine: Disallow specifying topology parameters as zero >>>> machine: Make smp_parse generic enough for all arches >>>> machine: Uniformly use maxcpus to calculate the omitted parameters >>>> machine: Use the computed parameters to calculate omitted cpus >>>> machine: Improve the error reporting of smp parsing >>>> hw: Add compat machines for 6.2 >>>> machine: Prefer cores over sockets in smp parsing since 6.2 >>>> machine: Use ms instead of global current_machine in sanity-check >>>> machine: Tweak the order of topology members in struct CpuTopology >>>> machine: Split out the smp parsing code >>>> tests/unit: Add a unit test for smp parsing >>>> >>>> MAINTAINERS | 2 + >>>> hw/arm/virt.c | 10 +- >>>> hw/core/machine-smp.c | 124 ++++ >>>> hw/core/machine.c | 68 +-- >>>> hw/core/meson.build | 1 + >>>> hw/i386/pc.c | 66 +-- >>>> hw/i386/pc_piix.c | 15 +- >>>> hw/i386/pc_q35.c | 14 +- >>>> hw/ppc/spapr.c | 16 +- >>>> hw/s390x/s390-virtio-ccw.c | 15 +- >>>> include/hw/boards.h | 13 +- >>>> include/hw/i386/pc.h | 3 + >>>> qapi/machine.json | 6 +- >>>> qemu-options.hx | 4 +- >>>> tests/unit/meson.build | 1 + >>>> tests/unit/test-smp-parse.c | 1117 +++++++++++++++++++++++++++++++++++ >>>> 16 files changed, 1338 insertions(+), 137 deletions(-) >>>> create mode 100644 hw/core/machine-smp.c >>>> create mode 100644 tests/unit/test-smp-parse.c >>>> > Please add me in CC as I reviewed one of the patch and was in middle > of reviewing few other patches > but missed the latest version. > Ok, I will, and thanks for the reviewing. v2 i.e. this version is now the latest. :) Thanks, Yanan
On 7/21/21 2:38 PM, wangyanan (Y) wrote: > On 2021/7/20 0:57, Cornelia Huck wrote: >> On Mon, Jul 19 2021, Yanan Wang <wangyanan55@huawei.com> wrote: >> >>> Hi, >>> >>> This is v2 of the series [1] that I have posted to introduce some smp >>> parsing >>> fixes and improvement, much more work has been processed compared to >>> RFC v1. >>> >>> [1] https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg00259.html >>> >>> The purpose of this series is to improve/fix the parsing logic. >>> Explicitly >>> specifying a CPU topology parameter as zero is not allowed any more, and >>> maxcpus is now uniformly used to calculate the omitted parameters. >>> It's also >>> suggested that we should start to prefer cores over sockets over >>> threads on >>> the newer machine types, which will make the computed virtual >>> topology more >>> reflective of the real hardware. >>> >>> In order to reduce code duplication and ease the code maintenance, >>> smp_parse >>> in now converted into a parser generic enough for all arches, so that >>> the PC >>> specific one can be removed. It's also convenient to introduce more >>> topology >>> members (e.g. cluster) to the generic parser in the future. >> Cc:ing Pierre, as he also had been looking at the smp parsing code (for >> s390x) recently. >> >> Also, please keep me on cc: for patches that touch s390x. > Sure, I will. Sorry about the missing. :) > > Thanks, > Yanan > . And me too please. Thanks Pierre
On 2021/7/22 15:51, Pierre Morel wrote: > > > On 7/21/21 2:38 PM, wangyanan (Y) wrote: >> On 2021/7/20 0:57, Cornelia Huck wrote: >>> On Mon, Jul 19 2021, Yanan Wang <wangyanan55@huawei.com> wrote: >>> >>>> Hi, >>>> >>>> This is v2 of the series [1] that I have posted to introduce some >>>> smp parsing >>>> fixes and improvement, much more work has been processed compared >>>> to RFC v1. >>>> >>>> [1] >>>> https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg00259.html >>>> >>>> The purpose of this series is to improve/fix the parsing logic. >>>> Explicitly >>>> specifying a CPU topology parameter as zero is not allowed any >>>> more, and >>>> maxcpus is now uniformly used to calculate the omitted parameters. >>>> It's also >>>> suggested that we should start to prefer cores over sockets over >>>> threads on >>>> the newer machine types, which will make the computed virtual >>>> topology more >>>> reflective of the real hardware. >>>> >>>> In order to reduce code duplication and ease the code maintenance, >>>> smp_parse >>>> in now converted into a parser generic enough for all arches, so >>>> that the PC >>>> specific one can be removed. It's also convenient to introduce more >>>> topology >>>> members (e.g. cluster) to the generic parser in the future. >>> Cc:ing Pierre, as he also had been looking at the smp parsing code (for >>> s390x) recently. >>> >>> Also, please keep me on cc: for patches that touch s390x. >> Sure, I will. Sorry about the missing. :) >> >> Thanks, >> Yanan >> . > And me too please. > Thanks > Pierre > Of course. Thanks, Yanan