Re: [PATCH 01/16] tests: test-smp-parse: Add the test for cores/threads per socket helpers

[Zhao Liu]

On Fri, Sep 15, 2023 at 02:31:38PM +0200, Igor Mammedov wrote:
> Date: Fri, 15 Sep 2023 14:31:38 +0200
> From: Igor Mammedov <imammedo@redhat.com>
> Subject: Re: [PATCH 01/16] tests: test-smp-parse: Add the test for
>  cores/threads per socket helpers
> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-redhat-linux-gnu)
> 
> On Fri, 25 Aug 2023 11:36:04 +0800
> Zhao Liu <zhao1.liu@linux.intel.com> wrote:
> 
> > From: Zhao Liu <zhao1.liu@intel.com>
> > 
> > Use the different ways to calculate cores/threads per socket, so that
> > the new CPU topology levels won't be missed in these 2 helpes:
> > 
> > * machine_topo_get_cores_per_socket()
> > * machine_topo_get_threads_per_socket()
> > 
> > Test the commit a1d027be95bc3 ("machine: Add helpers to get cores/
> > threads per socket").
> > 
> > Suggested-by: Igor Mammedov <imammedo@redhat.com>
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> 
> Acked-by: Igor Mammedov <imammedo@redhat.com>

Thanks!

-Zhao

> 
> > ---
> >  tests/unit/test-smp-parse.c | 67 ++++++++++++++++++++++++++++++-------
> >  1 file changed, 54 insertions(+), 13 deletions(-)
> > 
> > diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
> > index fdc39a846ca6..24972666a74d 100644
> > --- a/tests/unit/test-smp-parse.c
> > +++ b/tests/unit/test-smp-parse.c
> > @@ -394,20 +394,47 @@ static char *smp_config_to_string(const 
> > SMPConfiguration *config)
> >          config->has_maxcpus ? "true" : "false", config->maxcpus);
> >  }
> >  
> > -static char *cpu_topology_to_string(const CpuTopology *topo)
> > +/* Use the different calculation than 
> > machine_topo_get_threads_per_socket(). */
> > +static unsigned int cpu_topology_get_threads_per_socket(const CpuTopology 
> > *topo)
> > +{
> > +    /* Check the divisor to avoid invalid topology examples causing 
> > SIGFPE. */
> > +    if (!topo->sockets) {
> > +        return 0;
> > +    } else {
> > +        return topo->max_cpus / topo->sockets;
> > +    }
> > +}
> > +
> > +/* Use the different calculation than machine_topo_get_cores_per_socket(). 
> > */
> > +static unsigned int cpu_topology_get_cores_per_socket(const CpuTopology 
> > *topo)
> > +{
> > +    /* Check the divisor to avoid invalid topology examples causing 
> > SIGFPE. */
> > +    if (!topo->threads) {
> > +        return 0;
> > +    } else {
> > +        return cpu_topology_get_threads_per_socket(topo) / topo->threads;
> > +    }
> > +}
> > +
> > +static char *cpu_topology_to_string(const CpuTopology *topo,
> > +                                    unsigned int threads_per_socket,
> > +                                    unsigned int cores_per_socket)
> >  {
> >      return g_strdup_printf(
> >          "(CpuTopology) {\n"
> > -        "    .cpus     = %u,\n"
> > -        "    .sockets  = %u,\n"
> > -        "    .dies     = %u,\n"
> > -        "    .clusters = %u,\n"
> > -        "    .cores    = %u,\n"
> > -        "    .threads  = %u,\n"
> > -        "    .max_cpus = %u,\n"
> > +        "    .cpus               = %u,\n"
> > +        "    .sockets            = %u,\n"
> > +        "    .dies               = %u,\n"
> > +        "    .clusters           = %u,\n"
> > +        "    .cores              = %u,\n"
> > +        "    .threads            = %u,\n"
> > +        "    .max_cpus           = %u,\n"
> > +        "    .threads_per_socket = %u,\n"
> > +        "    .cores_per_socket   = %u,\n"
> >          "}",
> >          topo->cpus, topo->sockets, topo->dies, topo->clusters,
> > -        topo->cores, topo->threads, topo->max_cpus);
> > +        topo->cores, topo->threads, topo->max_cpus,
> > +        threads_per_socket, cores_per_socket);
> >  }
> >  
> >  static void check_parse(MachineState *ms, const SMPConfiguration *config,
> > @@ -415,14 +442,26 @@ static void check_parse(MachineState *ms, const 
> > SMPConfiguration *config,
> >                          bool is_valid)
> >  {
> >      g_autofree char *config_str = smp_config_to_string(config);
> > -    g_autofree char *expect_topo_str = cpu_topology_to_string(expect_topo);
> > -    g_autofree char *output_topo_str = NULL;
> > +    g_autofree char *expect_topo_str = NULL, *output_topo_str = NULL;
> > +    unsigned int expect_threads_per_socket, expect_cores_per_socket;
> > +    unsigned int ms_threads_per_socket, ms_cores_per_socket;
> >      Error *err = NULL;
> >  
> > +    expect_threads_per_socket =
> > +                        cpu_topology_get_threads_per_socket(expect_topo);
> > +    expect_cores_per_socket =
> > +                        cpu_topology_get_cores_per_socket(expect_topo);
> > +    expect_topo_str = cpu_topology_to_string(expect_topo,
> > +                                             expect_threads_per_socket,
> > +                                             expect_cores_per_socket);
> > +
> >      /* call the generic parser */
> >      machine_parse_smp_config(ms, config, &err);
> >  
> > -    output_topo_str = cpu_topology_to_string(&ms->smp);
> > +    ms_threads_per_socket = machine_topo_get_threads_per_socket(ms);
> > +    ms_cores_per_socket = machine_topo_get_cores_per_socket(ms);
> > +    output_topo_str = cpu_topology_to_string(&ms->smp, 
> > ms_threads_per_socket,
> > +                                             ms_cores_per_socket);
> >  
> >      /* when the configuration is supposed to be valid */
> >      if (is_valid) {
> > @@ -433,7 +472,9 @@ static void check_parse(MachineState *ms, const 
> > SMPConfiguration *config,
> >              (ms->smp.clusters == expect_topo->clusters) &&
> >              (ms->smp.cores == expect_topo->cores) &&
> >              (ms->smp.threads == expect_topo->threads) &&
> > -            (ms->smp.max_cpus == expect_topo->max_cpus)) {
> > +            (ms->smp.max_cpus == expect_topo->max_cpus) &&
> > +            (ms_threads_per_socket == expect_threads_per_socket) &&
> > +            (ms_cores_per_socket == expect_cores_per_socket)) {
> >              return;
> >          }
> >  
>