[PATCH] x86, reboot: skip reboot_fixups in early boot phase

[PATCH] x86, reboot: skip reboot_fixups in early boot phase

[Joonsoo Kim]

During early boot phase, PCI bus subsystem is not yet initialized.
If panic is occured in early boot phase and panic_timeout is set,
code flow go into emergency_restart() and hit mach_reboot_fixups(), then
encounter another panic. When second panic, we can't hold a panic_lock, so
code flow go into panic_smp_self_stop() which prevent system to restart.

For avoid second panic, skip reboot_fixups in early boot phase.
It makes panic_timeout works in early boot phase.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Joonsoo Kim <js1304@gmail.com>

diff --git a/arch/x86/kernel/reboot_fixups_32.c b/arch/x86/kernel/reboot_fixups_32.c
index c8e41e9..b9b8ec9 100644
--- a/arch/x86/kernel/reboot_fixups_32.c
+++ b/arch/x86/kernel/reboot_fixups_32.c
@@ -89,6 +89,10 @@ void mach_reboot_fixups(void)
 	if (in_interrupt())
 		return;
 
+	/* During early boot phase, PCI is not yet initialized */
+	if (system_state == SYSTEM_BOOTING)
+		return;
+
 	for (i=0; i < ARRAY_SIZE(fixups_table); i++) {
 		cur = &(fixups_table[i]);
 		dev = pci_get_device(cur->vendor, cur->device, NULL);
-- 
1.7.9.5

Re: [PATCH] x86, reboot: skip reboot_fixups in early boot phase

[Bjorn Helgaas]

On Fri, Dec 28, 2012 at 6:50 AM, Joonsoo Kim <js1304@gmail.com> wrote:
> During early boot phase, PCI bus subsystem is not yet initialized.
> If panic is occured in early boot phase and panic_timeout is set,
> code flow go into emergency_restart() and hit mach_reboot_fixups(), then
> encounter another panic. When second panic, we can't hold a panic_lock, so
> code flow go into panic_smp_self_stop() which prevent system to restart.
>
> For avoid second panic, skip reboot_fixups in early boot phase.
> It makes panic_timeout works in early boot phase.
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Signed-off-by: Joonsoo Kim <js1304@gmail.com>
>
> diff --git a/arch/x86/kernel/reboot_fixups_32.c b/arch/x86/kernel/reboot_fixups_32.c
> index c8e41e9..b9b8ec9 100644
> --- a/arch/x86/kernel/reboot_fixups_32.c
> +++ b/arch/x86/kernel/reboot_fixups_32.c
> @@ -89,6 +89,10 @@ void mach_reboot_fixups(void)
>         if (in_interrupt())
>                 return;
>
> +       /* During early boot phase, PCI is not yet initialized */
> +       if (system_state == SYSTEM_BOOTING)
> +               return;
> +
>         for (i=0; i < ARRAY_SIZE(fixups_table); i++) {
>                 cur = &(fixups_table[i]);
>                 dev = pci_get_device(cur->vendor, cur->device, NULL);

I guess you're saying that if we call pci_get_device() too early, it
panics?  Did you figure out why that happens?

If we call pci_get_device() before PCI has been initialized, it would
be good if it just returned NULL, indicating that we didn't find any
matching device.  I looked briefly, and I thought that's what would
happen, but apparently I'm missing something.

Bjorn

Re: [PATCH] x86, reboot: skip reboot_fixups in early boot phase

[Joonsoo Kim]

Hello, Bjorn.

On Thu, Jan 24, 2013 at 10:45:13AM -0700, Bjorn Helgaas wrote:
> On Fri, Dec 28, 2012 at 6:50 AM, Joonsoo Kim <js1304@gmail.com> wrote:
> > During early boot phase, PCI bus subsystem is not yet initialized.
> > If panic is occured in early boot phase and panic_timeout is set,
> > code flow go into emergency_restart() and hit mach_reboot_fixups(), then
> > encounter another panic. When second panic, we can't hold a panic_lock, so
> > code flow go into panic_smp_self_stop() which prevent system to restart.
> >
> > For avoid second panic, skip reboot_fixups in early boot phase.
> > It makes panic_timeout works in early boot phase.
> >
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Signed-off-by: Joonsoo Kim <js1304@gmail.com>
> >
> > diff --git a/arch/x86/kernel/reboot_fixups_32.c b/arch/x86/kernel/reboot_fixups_32.c
> > index c8e41e9..b9b8ec9 100644
> > --- a/arch/x86/kernel/reboot_fixups_32.c
> > +++ b/arch/x86/kernel/reboot_fixups_32.c
> > @@ -89,6 +89,10 @@ void mach_reboot_fixups(void)
> >         if (in_interrupt())
> >                 return;
> >
> > +       /* During early boot phase, PCI is not yet initialized */
> > +       if (system_state == SYSTEM_BOOTING)
> > +               return;
> > +
> >         for (i=0; i < ARRAY_SIZE(fixups_table); i++) {
> >                 cur = &(fixups_table[i]);
> >                 dev = pci_get_device(cur->vendor, cur->device, NULL);
> 
> I guess you're saying that if we call pci_get_device() too early, it
> panics?  Did you figure out why that happens?
> 
> If we call pci_get_device() before PCI has been initialized, it would
> be good if it just returned NULL, indicating that we didn't find any
> matching device.  I looked briefly, and I thought that's what would
> happen, but apparently I'm missing something.

In bus_find_device(), klist_iter_init_node() is called with 
@bus->p->klist_device. Before initialization, bus->p is NULL,
so panic is occured.

> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] x86, reboot: skip reboot_fixups in early boot phase

[Bjorn Helgaas]

[+cc Greg for driver core]

On Fri, Jan 25, 2013 at 10:13:03AM +0900, Joonsoo Kim wrote:
> Hello, Bjorn.
> 
> On Thu, Jan 24, 2013 at 10:45:13AM -0700, Bjorn Helgaas wrote:
> > On Fri, Dec 28, 2012 at 6:50 AM, Joonsoo Kim <js1304@gmail.com> wrote:
> > > During early boot phase, PCI bus subsystem is not yet initialized.
> > > If panic is occured in early boot phase and panic_timeout is set,
> > > code flow go into emergency_restart() and hit mach_reboot_fixups(), then
> > > encounter another panic. When second panic, we can't hold a panic_lock, so
> > > code flow go into panic_smp_self_stop() which prevent system to restart.
> > >
> > > For avoid second panic, skip reboot_fixups in early boot phase.
> > > It makes panic_timeout works in early boot phase.
> > >
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Ingo Molnar <mingo@redhat.com>
> > > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > > Signed-off-by: Joonsoo Kim <js1304@gmail.com>
> > >
> > > diff --git a/arch/x86/kernel/reboot_fixups_32.c b/arch/x86/kernel/reboot_fixups_32.c
> > > index c8e41e9..b9b8ec9 100644
> > > --- a/arch/x86/kernel/reboot_fixups_32.c
> > > +++ b/arch/x86/kernel/reboot_fixups_32.c
> > > @@ -89,6 +89,10 @@ void mach_reboot_fixups(void)
> > >         if (in_interrupt())
> > >                 return;
> > >
> > > +       /* During early boot phase, PCI is not yet initialized */
> > > +       if (system_state == SYSTEM_BOOTING)
> > > +               return;
> > > +
> > >         for (i=0; i < ARRAY_SIZE(fixups_table); i++) {
> > >                 cur = &(fixups_table[i]);
> > >                 dev = pci_get_device(cur->vendor, cur->device, NULL);
> > 
> > I guess you're saying that if we call pci_get_device() too early, it
> > panics?  Did you figure out why that happens?
> > 
> > If we call pci_get_device() before PCI has been initialized, it would
> > be good if it just returned NULL, indicating that we didn't find any
> > matching device.  I looked briefly, and I thought that's what would
> > happen, but apparently I'm missing something.
> 
> In bus_find_device(), klist_iter_init_node() is called with 
> @bus->p->klist_device. Before initialization, bus->p is NULL,
> so panic is occured.

I see.  pci_bus_type.p is initialized by __bus_register() in this path:

  pci_driver_init        # postcore_initcall
    bus_register(&pci_bus_type)
      __bus_register
        priv = kzalloc(sizeof(struct subsys_private))
	bus->p = priv
	klist_init(&priv->klist_devices, klist_devices_get, klist_devices_put)

I was hoping we could statically initialize the klist, but that doesn't
seem likely.

But I wonder if we could do something like the following.  If we could,
then callers wouldn't have to worry about whether or not the bus has been
initialized.

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 24eb078..ede19b8 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -290,7 +290,7 @@ int bus_for_each_dev(struct bus_type *bus, struct device *start,
 	struct device *dev;
 	int error = 0;
 
-	if (!bus)
+	if (!bus || !bus->p)
 		return -EINVAL;
 
 	klist_iter_init_node(&bus->p->klist_devices, &i,
@@ -324,7 +324,7 @@ struct device *bus_find_device(struct bus_type *bus,
 	struct klist_iter i;
 	struct device *dev;
 
-	if (!bus)
+	if (!bus || !bus->p)
 		return NULL;
 
 	klist_iter_init_node(&bus->p->klist_devices, &i,
@@ -440,7 +440,7 @@ int bus_for_each_drv(struct bus_type *bus, struct device_driver *start,
 	struct device_driver *drv;
 	int error = 0;
 
-	if (!bus)
+	if (!bus || !bus->p)
 		return -EINVAL;
 
 	klist_iter_init_node(&bus->p->klist_drivers, &i,

Re: [PATCH] x86, reboot: skip reboot_fixups in early boot phase

[Greg Kroah-Hartman]

On Thu, Jan 24, 2013 at 07:59:01PM -0700, Bjorn Helgaas wrote:
> [+cc Greg for driver core]
> 
> On Fri, Jan 25, 2013 at 10:13:03AM +0900, Joonsoo Kim wrote:
> > Hello, Bjorn.
> > 
> > On Thu, Jan 24, 2013 at 10:45:13AM -0700, Bjorn Helgaas wrote:
> > > On Fri, Dec 28, 2012 at 6:50 AM, Joonsoo Kim <js1304@gmail.com> wrote:
> > > > During early boot phase, PCI bus subsystem is not yet initialized.
> > > > If panic is occured in early boot phase and panic_timeout is set,
> > > > code flow go into emergency_restart() and hit mach_reboot_fixups(), then
> > > > encounter another panic. When second panic, we can't hold a panic_lock, so
> > > > code flow go into panic_smp_self_stop() which prevent system to restart.
> > > >
> > > > For avoid second panic, skip reboot_fixups in early boot phase.
> > > > It makes panic_timeout works in early boot phase.
> > > >
> > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > > Cc: Ingo Molnar <mingo@redhat.com>
> > > > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > > > Signed-off-by: Joonsoo Kim <js1304@gmail.com>
> > > >
> > > > diff --git a/arch/x86/kernel/reboot_fixups_32.c b/arch/x86/kernel/reboot_fixups_32.c
> > > > index c8e41e9..b9b8ec9 100644
> > > > --- a/arch/x86/kernel/reboot_fixups_32.c
> > > > +++ b/arch/x86/kernel/reboot_fixups_32.c
> > > > @@ -89,6 +89,10 @@ void mach_reboot_fixups(void)
> > > >         if (in_interrupt())
> > > >                 return;
> > > >
> > > > +       /* During early boot phase, PCI is not yet initialized */
> > > > +       if (system_state == SYSTEM_BOOTING)
> > > > +               return;
> > > > +
> > > >         for (i=0; i < ARRAY_SIZE(fixups_table); i++) {
> > > >                 cur = &(fixups_table[i]);
> > > >                 dev = pci_get_device(cur->vendor, cur->device, NULL);
> > > 
> > > I guess you're saying that if we call pci_get_device() too early, it
> > > panics?  Did you figure out why that happens?
> > > 
> > > If we call pci_get_device() before PCI has been initialized, it would
> > > be good if it just returned NULL, indicating that we didn't find any
> > > matching device.  I looked briefly, and I thought that's what would
> > > happen, but apparently I'm missing something.
> > 
> > In bus_find_device(), klist_iter_init_node() is called with 
> > @bus->p->klist_device. Before initialization, bus->p is NULL,
> > so panic is occured.
> 
> I see.  pci_bus_type.p is initialized by __bus_register() in this path:
> 
>   pci_driver_init        # postcore_initcall
>     bus_register(&pci_bus_type)
>       __bus_register
>         priv = kzalloc(sizeof(struct subsys_private))
> 	bus->p = priv
> 	klist_init(&priv->klist_devices, klist_devices_get, klist_devices_put)
> 
> I was hoping we could statically initialize the klist, but that doesn't
> seem likely.
> 
> But I wonder if we could do something like the following.  If we could,
> then callers wouldn't have to worry about whether or not the bus has been
> initialized.

<snip>

I have no objection to that patch, but really, someone wants to call
pci_find_device() before PCI is initialized?  Can't that be fixed
instead, as that is the root problem, not the driver core.

But, to paper over your subsystem's bugs, I guess I can take it :)

Care to resend it in a format that I can apply it in?

thanks,

greg k-h

Re: [PATCH] x86, reboot: skip reboot_fixups in early boot phase

[Bjorn Helgaas]

On Thu, Jan 24, 2013 at 9:14 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jan 24, 2013 at 07:59:01PM -0700, Bjorn Helgaas wrote:
>> [+cc Greg for driver core]
>>
>> On Fri, Jan 25, 2013 at 10:13:03AM +0900, Joonsoo Kim wrote:
>> > Hello, Bjorn.
>> >
>> > On Thu, Jan 24, 2013 at 10:45:13AM -0700, Bjorn Helgaas wrote:
>> > > On Fri, Dec 28, 2012 at 6:50 AM, Joonsoo Kim <js1304@gmail.com> wrote:
>> > > > During early boot phase, PCI bus subsystem is not yet initialized.
>> > > > If panic is occured in early boot phase and panic_timeout is set,
>> > > > code flow go into emergency_restart() and hit mach_reboot_fixups(), then
>> > > > encounter another panic. When second panic, we can't hold a panic_lock, so
>> > > > code flow go into panic_smp_self_stop() which prevent system to restart.
>> > > >
>> > > > For avoid second panic, skip reboot_fixups in early boot phase.
>> > > > It makes panic_timeout works in early boot phase.
>> > > >
>> > > > Cc: Thomas Gleixner <tglx@linutronix.de>
>> > > > Cc: Ingo Molnar <mingo@redhat.com>
>> > > > Cc: "H. Peter Anvin" <hpa@zytor.com>
>> > > > Signed-off-by: Joonsoo Kim <js1304@gmail.com>
>> > > >
>> > > > diff --git a/arch/x86/kernel/reboot_fixups_32.c b/arch/x86/kernel/reboot_fixups_32.c
>> > > > index c8e41e9..b9b8ec9 100644
>> > > > --- a/arch/x86/kernel/reboot_fixups_32.c
>> > > > +++ b/arch/x86/kernel/reboot_fixups_32.c
>> > > > @@ -89,6 +89,10 @@ void mach_reboot_fixups(void)
>> > > >         if (in_interrupt())
>> > > >                 return;
>> > > >
>> > > > +       /* During early boot phase, PCI is not yet initialized */
>> > > > +       if (system_state == SYSTEM_BOOTING)
>> > > > +               return;
>> > > > +
>> > > >         for (i=0; i < ARRAY_SIZE(fixups_table); i++) {
>> > > >                 cur = &(fixups_table[i]);
>> > > >                 dev = pci_get_device(cur->vendor, cur->device, NULL);
>> > >
>> > > I guess you're saying that if we call pci_get_device() too early, it
>> > > panics?  Did you figure out why that happens?
>> > >
>> > > If we call pci_get_device() before PCI has been initialized, it would
>> > > be good if it just returned NULL, indicating that we didn't find any
>> > > matching device.  I looked briefly, and I thought that's what would
>> > > happen, but apparently I'm missing something.
>> >
>> > In bus_find_device(), klist_iter_init_node() is called with
>> > @bus->p->klist_device. Before initialization, bus->p is NULL,
>> > so panic is occured.
>>
>> I see.  pci_bus_type.p is initialized by __bus_register() in this path:
>>
>>   pci_driver_init        # postcore_initcall
>>     bus_register(&pci_bus_type)
>>       __bus_register
>>         priv = kzalloc(sizeof(struct subsys_private))
>>       bus->p = priv
>>       klist_init(&priv->klist_devices, klist_devices_get, klist_devices_put)
>>
>> I was hoping we could statically initialize the klist, but that doesn't
>> seem likely.
>>
>> But I wonder if we could do something like the following.  If we could,
>> then callers wouldn't have to worry about whether or not the bus has been
>> initialized.
>
> <snip>
>
> I have no objection to that patch, but really, someone wants to call
> pci_find_device() before PCI is initialized?  Can't that be fixed
> instead, as that is the root problem, not the driver core.
>
> But, to paper over your subsystem's bugs, I guess I can take it :)

The caller is in the native_machine_emergency_restart() path.
Joonsoo's original patch does what I think you're suggesting:

>> > > > +       /* During early boot phase, PCI is not yet initialized */
>> > > > +       if (system_state == SYSTEM_BOOTING)
>> > > > +               return;
>> > > > +
>> > > >         for (i=0; i < ARRAY_SIZE(fixups_table); i++) {
>> > > >                 cur = &(fixups_table[i]);
>> > > >                 dev = pci_get_device(cur->vendor, cur->device, NULL);

I think it's sort of ugly to check system_state before using
pci_get_device(), and there's not really an obvious connection between
system_state and PCI initialization.

But if you prefer that, Joonsoo's original patch is fine with me.

Re: [PATCH] x86, reboot: skip reboot_fixups in early boot phase

[Greg Kroah-Hartman]

On Thu, Jan 24, 2013 at 09:21:52PM -0700, Bjorn Helgaas wrote:
> On Thu, Jan 24, 2013 at 9:14 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, Jan 24, 2013 at 07:59:01PM -0700, Bjorn Helgaas wrote:
> >> [+cc Greg for driver core]
> >>
> >> On Fri, Jan 25, 2013 at 10:13:03AM +0900, Joonsoo Kim wrote:
> >> > Hello, Bjorn.
> >> >
> >> > On Thu, Jan 24, 2013 at 10:45:13AM -0700, Bjorn Helgaas wrote:
> >> > > On Fri, Dec 28, 2012 at 6:50 AM, Joonsoo Kim <js1304@gmail.com> wrote:
> >> > > > During early boot phase, PCI bus subsystem is not yet initialized.
> >> > > > If panic is occured in early boot phase and panic_timeout is set,
> >> > > > code flow go into emergency_restart() and hit mach_reboot_fixups(), then
> >> > > > encounter another panic. When second panic, we can't hold a panic_lock, so
> >> > > > code flow go into panic_smp_self_stop() which prevent system to restart.
> >> > > >
> >> > > > For avoid second panic, skip reboot_fixups in early boot phase.
> >> > > > It makes panic_timeout works in early boot phase.
> >> > > >
> >> > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> >> > > > Cc: Ingo Molnar <mingo@redhat.com>
> >> > > > Cc: "H. Peter Anvin" <hpa@zytor.com>
> >> > > > Signed-off-by: Joonsoo Kim <js1304@gmail.com>
> >> > > >
> >> > > > diff --git a/arch/x86/kernel/reboot_fixups_32.c b/arch/x86/kernel/reboot_fixups_32.c
> >> > > > index c8e41e9..b9b8ec9 100644
> >> > > > --- a/arch/x86/kernel/reboot_fixups_32.c
> >> > > > +++ b/arch/x86/kernel/reboot_fixups_32.c
> >> > > > @@ -89,6 +89,10 @@ void mach_reboot_fixups(void)
> >> > > >         if (in_interrupt())
> >> > > >                 return;
> >> > > >
> >> > > > +       /* During early boot phase, PCI is not yet initialized */
> >> > > > +       if (system_state == SYSTEM_BOOTING)
> >> > > > +               return;
> >> > > > +
> >> > > >         for (i=0; i < ARRAY_SIZE(fixups_table); i++) {
> >> > > >                 cur = &(fixups_table[i]);
> >> > > >                 dev = pci_get_device(cur->vendor, cur->device, NULL);
> >> > >
> >> > > I guess you're saying that if we call pci_get_device() too early, it
> >> > > panics?  Did you figure out why that happens?
> >> > >
> >> > > If we call pci_get_device() before PCI has been initialized, it would
> >> > > be good if it just returned NULL, indicating that we didn't find any
> >> > > matching device.  I looked briefly, and I thought that's what would
> >> > > happen, but apparently I'm missing something.
> >> >
> >> > In bus_find_device(), klist_iter_init_node() is called with
> >> > @bus->p->klist_device. Before initialization, bus->p is NULL,
> >> > so panic is occured.
> >>
> >> I see.  pci_bus_type.p is initialized by __bus_register() in this path:
> >>
> >>   pci_driver_init        # postcore_initcall
> >>     bus_register(&pci_bus_type)
> >>       __bus_register
> >>         priv = kzalloc(sizeof(struct subsys_private))
> >>       bus->p = priv
> >>       klist_init(&priv->klist_devices, klist_devices_get, klist_devices_put)
> >>
> >> I was hoping we could statically initialize the klist, but that doesn't
> >> seem likely.
> >>
> >> But I wonder if we could do something like the following.  If we could,
> >> then callers wouldn't have to worry about whether or not the bus has been
> >> initialized.
> >
> > <snip>
> >
> > I have no objection to that patch, but really, someone wants to call
> > pci_find_device() before PCI is initialized?  Can't that be fixed
> > instead, as that is the root problem, not the driver core.
> >
> > But, to paper over your subsystem's bugs, I guess I can take it :)
> 
> The caller is in the native_machine_emergency_restart() path.
> Joonsoo's original patch does what I think you're suggesting:
> 
> >> > > > +       /* During early boot phase, PCI is not yet initialized */
> >> > > > +       if (system_state == SYSTEM_BOOTING)
> >> > > > +               return;
> >> > > > +
> >> > > >         for (i=0; i < ARRAY_SIZE(fixups_table); i++) {
> >> > > >                 cur = &(fixups_table[i]);
> >> > > >                 dev = pci_get_device(cur->vendor, cur->device, NULL);
> 
> I think it's sort of ugly to check system_state before using
> pci_get_device(), and there's not really an obvious connection between
> system_state and PCI initialization.
> 
> But if you prefer that, Joonsoo's original patch is fine with me.

Both probably would be best, as there are probably other things that you
don't want to touch when you are still booting and trying to restart the
machine at the same time.

thanks,

greg k-h

Re: [PATCH] x86, reboot: skip reboot_fixups in early boot phase

[Joonsoo Kim]

On Thu, Jan 24, 2013 at 08:52:32PM -0800, Greg Kroah-Hartman wrote:
> On Thu, Jan 24, 2013 at 09:21:52PM -0700, Bjorn Helgaas wrote:
> > On Thu, Jan 24, 2013 at 9:14 PM, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > > On Thu, Jan 24, 2013 at 07:59:01PM -0700, Bjorn Helgaas wrote:
> > >> [+cc Greg for driver core]
> > >>
> > >> On Fri, Jan 25, 2013 at 10:13:03AM +0900, Joonsoo Kim wrote:
> > >> > Hello, Bjorn.
> > >> >
> > >> > On Thu, Jan 24, 2013 at 10:45:13AM -0700, Bjorn Helgaas wrote:
> > >> > > On Fri, Dec 28, 2012 at 6:50 AM, Joonsoo Kim <js1304@gmail.com> wrote:
> > >> > > > During early boot phase, PCI bus subsystem is not yet initialized.
> > >> > > > If panic is occured in early boot phase and panic_timeout is set,
> > >> > > > code flow go into emergency_restart() and hit mach_reboot_fixups(), then
> > >> > > > encounter another panic. When second panic, we can't hold a panic_lock, so
> > >> > > > code flow go into panic_smp_self_stop() which prevent system to restart.
> > >> > > >
> > >> > > > For avoid second panic, skip reboot_fixups in early boot phase.
> > >> > > > It makes panic_timeout works in early boot phase.
> > >> > > >
> > >> > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > >> > > > Cc: Ingo Molnar <mingo@redhat.com>
> > >> > > > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > >> > > > Signed-off-by: Joonsoo Kim <js1304@gmail.com>
> > >> > > >
> > >> > > > diff --git a/arch/x86/kernel/reboot_fixups_32.c b/arch/x86/kernel/reboot_fixups_32.c
> > >> > > > index c8e41e9..b9b8ec9 100644
> > >> > > > --- a/arch/x86/kernel/reboot_fixups_32.c
> > >> > > > +++ b/arch/x86/kernel/reboot_fixups_32.c
> > >> > > > @@ -89,6 +89,10 @@ void mach_reboot_fixups(void)
> > >> > > >         if (in_interrupt())
> > >> > > >                 return;
> > >> > > >
> > >> > > > +       /* During early boot phase, PCI is not yet initialized */
> > >> > > > +       if (system_state == SYSTEM_BOOTING)
> > >> > > > +               return;
> > >> > > > +
> > >> > > >         for (i=0; i < ARRAY_SIZE(fixups_table); i++) {
> > >> > > >                 cur = &(fixups_table[i]);
> > >> > > >                 dev = pci_get_device(cur->vendor, cur->device, NULL);
> > >> > >
> > >> > > I guess you're saying that if we call pci_get_device() too early, it
> > >> > > panics?  Did you figure out why that happens?
> > >> > >
> > >> > > If we call pci_get_device() before PCI has been initialized, it would
> > >> > > be good if it just returned NULL, indicating that we didn't find any
> > >> > > matching device.  I looked briefly, and I thought that's what would
> > >> > > happen, but apparently I'm missing something.
> > >> >
> > >> > In bus_find_device(), klist_iter_init_node() is called with
> > >> > @bus->p->klist_device. Before initialization, bus->p is NULL,
> > >> > so panic is occured.
> > >>
> > >> I see.  pci_bus_type.p is initialized by __bus_register() in this path:
> > >>
> > >>   pci_driver_init        # postcore_initcall
> > >>     bus_register(&pci_bus_type)
> > >>       __bus_register
> > >>         priv = kzalloc(sizeof(struct subsys_private))
> > >>       bus->p = priv
> > >>       klist_init(&priv->klist_devices, klist_devices_get, klist_devices_put)
> > >>
> > >> I was hoping we could statically initialize the klist, but that doesn't
> > >> seem likely.
> > >>
> > >> But I wonder if we could do something like the following.  If we could,
> > >> then callers wouldn't have to worry about whether or not the bus has been
> > >> initialized.
> > >
> > > <snip>
> > >
> > > I have no objection to that patch, but really, someone wants to call
> > > pci_find_device() before PCI is initialized?  Can't that be fixed
> > > instead, as that is the root problem, not the driver core.
> > >
> > > But, to paper over your subsystem's bugs, I guess I can take it :)
> > 
> > The caller is in the native_machine_emergency_restart() path.
> > Joonsoo's original patch does what I think you're suggesting:
> > 
> > >> > > > +       /* During early boot phase, PCI is not yet initialized */
> > >> > > > +       if (system_state == SYSTEM_BOOTING)
> > >> > > > +               return;
> > >> > > > +
> > >> > > >         for (i=0; i < ARRAY_SIZE(fixups_table); i++) {
> > >> > > >                 cur = &(fixups_table[i]);
> > >> > > >                 dev = pci_get_device(cur->vendor, cur->device, NULL);
> > 
> > I think it's sort of ugly to check system_state before using
> > pci_get_device(), and there's not really an obvious connection between
> > system_state and PCI initialization.
> > 
> > But if you prefer that, Joonsoo's original patch is fine with me.
> 
> Both probably would be best, as there are probably other things that you
> don't want to touch when you are still booting and trying to restart the
> machine at the same time.

I agree that my patch is ugly :)
So I drop it and wait for your patch.

Thanks.