Re: [PATCH] PM/reboot: call syscore_shutdown() after disable_nonboot_cpus()

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Sunday, April 07, 2013 08:29:32 AM Greg KH wrote:
> On Sun, Apr 07, 2013 at 10:46:00AM +0200, Rafael J. Wysocki wrote:
> > On Sunday, April 07, 2013 10:14:14 AM Huacai Chen wrote:
> > > As commit 40dc166c (PM / Core: Introduce struct syscore_ops for core
> > > subsystems PM) say, syscore_ops operations should be carried with one
> > > CPU on-line and interrupts disabled. However, after commit f96972f2d
> > > (kernel/sys.c: call disable_nonboot_cpus() in kernel_restart()),
> > > syscore_shutdown() is called before disable_nonboot_cpus(), so break
> > > the rules. We have a MIPS machine with a 8259A PIC, and there is an
> > > external timer (HPET) linked at 8259A. Since 8259A has been shutdown
> > > too early (by syscore_shutdown()), disable_nonboot_cpus() runs without
> > > timer interrupt, so it hangs and reboot fails. This patch call
> > > syscore_shutdown() a little later (after disable_nonboot_cpus()) to
> > > avoid reboot failure, this is the same way as poweroff does.
> > > 
> > > BTW, add disable_nonboot_cpus() in kernel_halt() for consistency.
> > > 
> > > Signed-off-by: Huacai Chen <chenhc@lemote.com>
> > > Cc: <stable@vger.kernel.org>
> > 
> > While I agree with the changes, I'm not sure if I'm the right maintainer,
> > as this isn't really PM code.
> > 
> > Andrew, should I take this?
> 
> Andrew is on vacation for a few weeks, so you might need to take this
> through your tree.

OK

But, it looks like we should actually disable interrupts on the remaining
CPU after we've called disable_nonboot_cpus() so that the syscore_shutdown()
assumptions are satisfied which the patch doesn't do.

Chen (I apologize if that's not the right part of your full name to use here),
do you think that's not necessary and if so, then for what reason?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.