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

[David Gibson]

On Thu, Nov 07, 2019 at 06:12:49PM +0100, Greg Kurz wrote:
> 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

Ok, almost certainly a kernel side bug then.

> - 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 think I had an RH downstream bug about this issue that I hadn't yet
had a chance to investigate.  I'm away until the 19th, but if you
could co-ordinate with Laurent Vivier so we can get this fixed both
upstream and down, that would be great.

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



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