Re: [PATCH] spapr: Fix VSMT mode when it is not supported by the kernel

[David Gibson]

On Tue, Nov 19, 2019 at 05:13:29PM +0100, Lukáš Doktor wrote:
> Dne 19. 11. 19 v 16:45 Greg Kurz napsal(a):
> > On Tue, 19 Nov 2019 15:06:51 +0100
> > Laurent Vivier <address@hidden> wrote:
> > 
> >> On 19/11/2019 02:00, David Gibson wrote:
> >>> On Fri, Nov 08, 2019 at 05:47:59PM +0100, Greg Kurz wrote:
> >>>> On Fri,  8 Nov 2019 16:40:35 +0100
> >>>> Laurent Vivier <address@hidden> wrote:
> >>>>
> >>>>> Commit 29cb4187497d sets by default the VSMT to smp_threads,
> >>>>> but older kernels (< 4.13) don't support that.
> >>>>>
> >>>>> We can reasonably restore previous behavior with this kernel
> >>>>> to allow to run QEMU as before.
> >>>>>
> >>>>> If VSMT is not supported, VSMT will be set to MAX(8, smp_threads)
> >>>>> as it is done for previous machine types (< pseries-4.2)
> >>>>>
> >>>>
> >>>> It is usually _bad_ to base the machine behavior on host capabilities.
> >>>> What happens if we migrate between an older kernel and a recent one ?
> >>>
> >>> Right.  We're really trying to remove instaces of such behaviour.  I'd
> >>> prefer to completely revert Greg's original patch than to re-introduce
> >>> host configuration dependency into the guest configuration..
> >>>
> >>>> I understand this is to fix tests/migration-test on older kernels.
> >>>> Couldn't this be achieved with migration-test doing some introspection
> >>>> and maybe pass vsmt=8 on the QEMU command line ?
> >>>
> >>> ..adjusting the test case like this might be a better idea, though.
> >>>
> >>> What's the test setup where we're using the old kernel?  I really only
> >>> applied the original patch on the guess that we didn't really care
> >>> about kernels that old.  The fact you've hit this in practice makes me
> >>> doubt that assumption.
> >>>
> >>
> >> The way to fix the tests is to add "-smp threads=8" on the command line
> >> (for all tests, so basically in qtest_init_without_qmp_handshake(), and
> >> it will impact all the machine types), and we have to check if it is
> > 
> > Ohhh... it isn't possible to initialize Qtest with machine specific
> > properties ? That's a bit unfortunate :-\
> > 
> >> ppc64/pseries to do that, and there it becomes a little bit complicated
> >> for a so small piece of code.
> >>
> >> So I think the best to do is to revert Greg's patch.
> >>
> > 
> > I'm okay with that since this patch doesn't bring much for the moment.
> > 
> > But soon, ie. linux-5.5 hopefully, KVM will allow to configure the number
> > of presenters in the XIVE and XICS-on-XIVE devices on POWER9. Combined
> > with this patch, it will allow to drastically reduce the consumption of
> > resources in the XIVE HW, which currently limits the number of VMs that
> > can run concurrently with an in-kernel irqchip. So I hope the 'make check'
> > you're willing to fix is worth it :-), and BTW you didn't answer David's
> > question about the test setup.
> > 
> 
> Hello Greg, guys,
> 
> it is P8 machine using RHEL7 host with a stock kernel-3.10.0-XXXX. If you're 
> interested in the whole setup, then:
> 
>     ./configure --enable-kvm 
> --block-drv-rw-whitelist=vmdk,null-aio,quorum,null-co,blkverify,file,nbd,raw,blkdebug,host_device,qed,nbd,iscsi,gluster,rbd,qcow2,throttle,copy-on-read
>     make -j 13
>     SPEED=slow make check

Really the only relevant factor here is that it's a RHEL7 host, which
has an old kernel (3.10 is misleading, since it has a *heap* of
backports, but support for the vsmt feature isn't one of them).

So more generally than this specific test, the question really is
whether we thinkg RHEL7 is still in wide enough use that we care to
keep supporting it with the latest upstream machine type.  I'm a bit
undecided on that.

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

signature.asc
Description: PGP signature