Re: [PATCH v8 2/5] qtest/numa-test: Specify CPU topology in aarch64_numa_cpu()

[Gavin Shan]

Hi Igor,

On 5/3/22 4:54 PM, Igor Mammedov wrote:
> On Mon, 2 May 2022 18:07:00 +0800
> Gavin Shan <gshan@redhat.com> wrote:
> > On 5/2/22 4:52 PM, Igor Mammedov wrote:
> > > On Mon, 25 Apr 2022 11:27:59 +0800
> > > Gavin Shan <gshan@redhat.com> wrote:
> > >    
> > > > The CPU topology isn't enabled on arm/virt machine yet, but we're
> > > > going to do it in next patch. After the CPU topology is enabled by
> > > > next patch, "thrad-id=1" becomes invalid because the CPU core is
> > >                    ^^^ typo
> > >    
> >
> > hmm, my bad. Lets fix it in next revision.
> >
> > > > preferred on arm/virt machine. It means these two CPUs have 0/1
> > > > as their core IDs, but their thread IDs are all 0. It will trigger
> > > > test failure as the following message indicates:
> > > >
> > > >     [14/21 qemu:qtest+qtest-aarch64 / qtest-aarch64/numa-test  ERROR
> > > >     1.48s   killed by signal 6 SIGABRT
> > > >     >>> 
> > > > G_TEST_DBUS_DAEMON=/home/gavin/sandbox/qemu.main/tests/dbus-vmstate-daemon.sh \
> > > >         QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon   
> > > >       \
> > > >         QTEST_QEMU_BINARY=./qemu-system-aarch64                                 
> > > >       \
> > > >         QTEST_QEMU_IMG=./qemu-img MALLOC_PERTURB_=83                            
> > > >       \
> > > >         /home/gavin/sandbox/qemu.main/build/tests/qtest/numa-test --tap -k
> > > >     ――――――――――――――――――――――――――――――――――――――――――――――
> > > >     stderr:
> > > >     qemu-system-aarch64: -numa cpu,node-id=0,thread-id=1: no match found
> > > >
> > > > This fixes the issue by providing comprehensive SMP configurations
> > > > in aarch64_numa_cpu(). The SMP configurations aren't used before
> > > > the CPU topology is enabled in next patch.
> > > >
> > > > Signed-off-by: Gavin Shan <gshan@redhat.com>
> > > > Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
> > > > ---
> > > >    tests/qtest/numa-test.c | 3 ++-
> > > >    1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
> > > > index 90bf68a5b3..aeda8c774c 100644
> > > > --- a/tests/qtest/numa-test.c
> > > > +++ b/tests/qtest/numa-test.c
> > > > @@ -223,7 +223,8 @@ static void aarch64_numa_cpu(const void *data)
> > > >        QTestState *qts;
> > > >        g_autofree char *cli = NULL;
> > > >    
> > > > -    cli = make_cli(data, "-machine smp.cpus=2 "
> > > > +    cli = make_cli(data, "-machine "
> > > > +        "smp.cpus=2,smp.sockets=1,smp.clusters=1,smp.cores=1,smp.threads=2 "
> > > >            "-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
> > > >            "-numa cpu,node-id=1,thread-id=0 "
> > >                                   ^^^^
> > > make it sensible as well, i.e. socket/cluster/cores-ids ...
> > >    
> >
> > Could you help if the following command lines are what you want? I don't
> > think we can do it. Without PATCH[v8 3/5] applied, {socket,cluster,core}-id
> > are invalid from arm/virt machine side and we will run into errors.
> you are right, you can only fix -numa after 3/5
>
> btw:
> splitting threads between several numa nodes here probably is unreal
> configuration. Should be fixed in follow up patches.

Lets fix it in PATCH[v9 4/6] with the following command lines. Besides,
socket/cluster/core/thread IDs should be checked when the NUMA IDs are
verified. It helps to check if the CPU topology is properly populated
at least.

v9 should be sent shortly after doing some tests. Please take a look.

    -machine                                                           \
    smp.cpus=2,smp.sockets=2,smp.clusters=1,smp.cores=1,smp.threads=1  \
    -numa node,nodeid=0,memdev=ram -numa node,nodeid=1                 \
    -numa cpu,node-id=0,socket-id=1,cluster-id=0,core-id=0,thread-id=0 \
    -numa cpu,node-id=1,socket-id=0,cluster-id=0,core-id=0,thread-id=0

[...]

Thanks,
Gavin