Re: [RFC] powerpc/pseries: Increase busy loop in pseries_cpu_die

[Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>]

Am Dienstag, 7. Februar 2017, 08:26:45 BRST schrieb Balbir Singh:
> On Mon, Feb 06, 2017 at 04:58:16PM -0200, Thiago Jung Bauermann wrote:
> > [  447.714064] Querying DEAD? cpu 134 (134) shows 2
> > cpu 0x86: Vector: 300 (Data Access) at [c000000007b0fd40]
> > 
> >     pc: 000000001ec3072c
> >     lr: 000000001ec2fee0
> >     sp: 1faf6bd0
> >    
> >    msr: 8000000102801000
> >    dar: 212d6c1a2a20c
> 
> This looks like we accessed a bad address, but why?

Am Dienstag, 7. Februar 2017, 13:10:22 BRST schrieb Michael Ellerman:
> We shouldn't be crashing.
> 
> So we need to fix that.
> 
> We may also need to increase the timeout, though it's pretty gross TBH.
> 
> But step one is make sure we don't crash.

I didn't analyze exactly what is causing the CPU to crash because the root 
cause is the inconsistency between what the kernel thinks the CPU state is and 
reality. But if we have to be able to handle that inconsistency I will keep 
digging and try to fix that.

> > --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> > +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> > @@ -206,7 +206,7 @@ static void pseries_cpu_die(unsigned int cpu)
> > 
> >  		}
> >  	
> >  	} else if (get_preferred_offline_state(cpu) == CPU_STATE_OFFLINE) {
> > 
> > -		for (tries = 0; tries < 25; tries++) {
> > +		for (tries = 0; tries < 5000; tries++) {
> 
> This fixes some of the asymmetry between handling of CPU_STATE_INACTIVE
> and CPU_STATE_OFFLINE, but I think we can probably move the cpu_relax()
> to msleep(1).

I didn't change it to msleep() because I thought it would introduce a 
regression. commit b906cfa397fd ("powerpc/pseries: Fix cpu hotplug") changed a 
msleep(200) that was there to a cpu_relax() with this explanation:

    Currently, pseries_cpu_die() calls msleep() while polling RTAS for 
    the status of the dying cpu.

    However, if the cpu that is going down also happens to be the one 
    doing the tick then we're hosed as the tick_do_timer_cpu 'baton' is
    only passed later on in tick_shutdown() when _cpu_down() does the 
    CPU_DEAD notification.  Therefore jiffies won't be updated anymore.

    This replaces that msleep() with a cpu_relax() to make sure we're not 
    going to schedule at that point.

    With this patch my test box survives a 100k iterations hotplug stress
    test on _all_ cpus, whereas without it, it quickly dies after ~50 
    iterations.

I can try to add it back and see what happens. Perhaps that situation won't 
happen anymore with today's kernel.

> Please also see
> 940ce42 powerpc/pseries: Increase cpu die timeout

Yes, that is the commit that I mentioned in the patch description.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center