Re: [PATCH] spapr: change default interrupt mode back to 'xics'

[Greg Kurz]

On Wed, 6 Nov 2019 22:36:24 +0100
David Gibson <address@hidden> wrote:

> On Wed, Nov 06, 2019 at 09:28:20AM +0100, Greg Kurz wrote:
> > On Tue,  5 Nov 2019 15:17:44 -0600
> > Lijun Pan <address@hidden> wrote:
> > 
> > > The hugetlbfs hugepage leakage is found under following test procedures:
> > > 1. configure hugetlbfs to back guest memory with '-mem-path 
> > > /dev/hugepages'
> > > 2. hot plug a vcpu and remove it, then quit the guest.
> > > 3. hugepage leaks found on the host
> > > 
> > > git bisect points to the first bad commit: bd94bc064.
> > > 
> > 
> > Hmmm... Have you investigated some more to understand what's really
> > happening here ? Is it a problem with QEMU or KVM ? Does it also happen
> > if you don't use the in-kernel irqchip ?
> > 
> > Please provide more details because reverting to XICS looks like a big
> > hammer/workaround to me.
> 
> Quite.  This also changes the behaviour for the existing pseries-4.1
> machine type, which is absolutely a no go.
> 

Yeah I didn't even mention that. This workaround would need some
more care in order to be applied to newer machine types only.

> We need to fix this properly.
> 

Right. I've started to have a look. I could easily reproduce the issue with
the scenario described above. A few more details though:
- doesn't happen with kernel-irqchip=off
- happens with ic-mode=xive or ic-mode=dual, not with ic-mode=xics
- only happens when removing a vCPU, either cold plugged or hot plugged

Note that the leak can also be observed by resetting the machine instead
of removing a vCPU. The leak is one huge page (2M in my setup) per vCPU.

I still need to do some more digging but I'm starting to suspect EQs
which are allocated by the guest on a per-CPU basis.

> > 
> > > Fixes: bd94bc064 ("spapr: change default interrupt mode to 'dual'")
> > > Cc: Cédric Le Goater <address@hidden>
> > > Cc: David Gibson <address@hidden>
> > > Signed-off-by: Lijun Pan <address@hidden>
> > > ---
> > >  hw/ppc/spapr.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 94f9d27096..26b064fb6c 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -4437,7 +4437,7 @@ static void spapr_machine_class_init(ObjectClass 
> > > *oc, void *data)
> > >      smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
> > >      smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF;
> > >      spapr_caps_add_properties(smc, &error_abort);
> > > -    smc->irq = &spapr_irq_dual;
> > > +    smc->irq = &spapr_irq_xics;
> > >      smc->dr_phb_enabled = true;
> > >      smc->linux_pci_probe = true;
> > >      smc->smp_threads_vsmt = true;
> > 
>

pgpeqT8cfvlwO.pgp
Description: OpenPGP digital signature