2005-04-21 11:17:33

by Pavel Machek

[permalink] [raw]
Subject: [patch] properly stop devices before poweroff


Without this patch, Linux provokes emergency disk shutdowns and
similar nastiness. It was in SuSE kernels for some time, IIRC.

Signed-off-by: Pavel Machek <[email protected]>

--- clean/kernel/sys.c 2005-03-19 00:32:32.000000000 +0100
+++ linux/kernel/sys.c 2005-03-22 12:20:53.000000000 +0100
@@ -404,6 +404,7 @@
case LINUX_REBOOT_CMD_HALT:
notifier_call_chain(&reboot_notifier_list, SYS_HALT, NULL);
system_state = SYSTEM_HALT;
+ device_suspend(PMSG_SUSPEND);
device_shutdown();
printk(KERN_EMERG "System halted.\n");
machine_halt();
@@ -414,6 +415,7 @@
case LINUX_REBOOT_CMD_POWER_OFF:
notifier_call_chain(&reboot_notifier_list, SYS_POWER_OFF, NULL);
system_state = SYSTEM_POWER_OFF;
+ device_suspend(PMSG_SUSPEND);
device_shutdown();
printk(KERN_EMERG "Power down.\n");
machine_power_off();
@@ -430,6 +432,7 @@

notifier_call_chain(&reboot_notifier_list, SYS_RESTART, buffer);
system_state = SYSTEM_RESTART;
+ device_suspend(PMSG_FREEZE);
device_shutdown();
printk(KERN_EMERG "Restarting system with command '%s'.\n", buffer);
machine_restart(buffer);

--
Boycott Kodak -- for their patent abuse against Java.


2005-04-21 11:31:55

by Tvrtko Ursulin

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

On 21/04/2005 12:13:46 linux-kernel-owner wrote:

>Without this patch, Linux provokes emergency disk shutdowns and
>similar nastiness. It was in SuSE kernels for some time, IIRC.

OMG! And I did try to raise that issue 10 months ago, see below:

http://www.ussg.iu.edu/hypermail/linux/kernel/0406.0/0242.html


2005-04-21 11:38:48

by Pavel Machek

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

Hi!

> >Without this patch, Linux provokes emergency disk shutdowns and
> >similar nastiness. It was in SuSE kernels for some time, IIRC.
>
> OMG! And I did try to raise that issue 10 months ago, see below:
>
> http://www.ussg.iu.edu/hypermail/linux/kernel/0406.0/0242.html

Heh, verify that this patch works for you and it might finally get
fixed...

Pavel
--
Boycott Kodak -- for their patent abuse against Java.

2005-04-21 15:02:31

by Tvrtko Ursulin

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

On 21/04/2005 12:38:29 linux-kernel-owner wrote:

>> >Without this patch, Linux provokes emergency disk shutdowns and
>> >similar nastiness. It was in SuSE kernels for some time, IIRC.
>> OMG! And I did try to raise that issue 10 months ago, see below:
>> http://www.ussg.iu.edu/hypermail/linux/kernel/0406.0/0242.html
>
>Heh, verify that this patch works for you and it might finally get
>fixed...

I can't "hear" the difference between normal and emergency shutdown, and
information I found does not suggest they are recorded amongst s.m.a.r.t
attributes. Power_Cycle_Count, Load_Cycle_Count and
Power-Off_Retract_Count seem to be in sync, although common sense would
suggests that the last one should possibly not increment on a clean
shutdown. I will experiment a bit when I find some spare time.

2005-04-29 13:19:27

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

Pavel Machek <[email protected]> wrote:
>
>
> Without this patch, Linux provokes emergency disk shutdowns and
> similar nastiness. It was in SuSE kernels for some time, IIRC.
>

With this patch when running `halt -p' my ia64 Tiger (using
tiger_defconfig) gets a stream of badnesses in iosapic_unregister_intr()
and then hangs up.

Unfortunately it all seems to happen after the serial port has been
disabled because nothing comes out. I set the console to a squitty font
and took a piccy. See
http://www.zip.com.au/~akpm/linux/patches/stuff/dsc02505.jpg

I guess it's an ia64 problem. I'll leave the patch in -mm for now.

>
> --- clean/kernel/sys.c 2005-03-19 00:32:32.000000000 +0100
> +++ linux/kernel/sys.c 2005-03-22 12:20:53.000000000 +0100
> @@ -404,6 +404,7 @@
> case LINUX_REBOOT_CMD_HALT:
> notifier_call_chain(&reboot_notifier_list, SYS_HALT, NULL);
> system_state = SYSTEM_HALT;
> + device_suspend(PMSG_SUSPEND);
> device_shutdown();
> printk(KERN_EMERG "System halted.\n");
> machine_halt();
> @@ -414,6 +415,7 @@
> case LINUX_REBOOT_CMD_POWER_OFF:
> notifier_call_chain(&reboot_notifier_list, SYS_POWER_OFF, NULL);
> system_state = SYSTEM_POWER_OFF;
> + device_suspend(PMSG_SUSPEND);
> device_shutdown();
> printk(KERN_EMERG "Power down.\n");
> machine_power_off();
> @@ -430,6 +432,7 @@
>
> notifier_call_chain(&reboot_notifier_list, SYS_RESTART, buffer);
> system_state = SYSTEM_RESTART;
> + device_suspend(PMSG_FREEZE);
> device_shutdown();
> printk(KERN_EMERG "Restarting system with command '%s'.\n", buffer);
> machine_restart(buffer);
>
> --
> Boycott Kodak -- for their patent abuse against Java.

2005-04-29 13:33:53

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

On Fri, 29 Apr 2005, Andrew Morton wrote:

> Pavel Machek <[email protected]> wrote:
> >
> >
> > Without this patch, Linux provokes emergency disk shutdowns and
> > similar nastiness. It was in SuSE kernels for some time, IIRC.
> >
>
> With this patch when running `halt -p' my ia64 Tiger (using
> tiger_defconfig) gets a stream of badnesses in iosapic_unregister_intr()
> and then hangs up.
>
> Unfortunately it all seems to happen after the serial port has been
> disabled because nothing comes out. I set the console to a squitty font
> and took a piccy. See
> http://www.zip.com.au/~akpm/linux/patches/stuff/dsc02505.jpg
>
> I guess it's an ia64 problem. I'll leave the patch in -mm for now.

Could you cat /proc/interrupts during runtime? It looks like the device
being suspended never went through pci_device_enable() (e.g. ethernet
interface wasn't up). It's harmless right now.

Thanks,
Zwane

2005-04-30 09:48:36

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

Zwane Mwaikambo <[email protected]> wrote:
>
> On Fri, 29 Apr 2005, Andrew Morton wrote:
>
> > Pavel Machek <[email protected]> wrote:
> > >
> > >
> > > Without this patch, Linux provokes emergency disk shutdowns and
> > > similar nastiness. It was in SuSE kernels for some time, IIRC.
> > >
> >
> > With this patch when running `halt -p' my ia64 Tiger (using
> > tiger_defconfig) gets a stream of badnesses in iosapic_unregister_intr()
> > and then hangs up.
> >
> > Unfortunately it all seems to happen after the serial port has been
> > disabled because nothing comes out. I set the console to a squitty font
> > and took a piccy. See
> > http://www.zip.com.au/~akpm/linux/patches/stuff/dsc02505.jpg
> >
> > I guess it's an ia64 problem. I'll leave the patch in -mm for now.
>
> Could you cat /proc/interrupts during runtime?

CPU0 CPU1 CPU2 CPU3
28: 0 0 0 0 LSAPIC cpe_poll
29: 0 0 0 0 LSAPIC cmc_poll
30: 0 0 0 0 IO-SAPIC-level cpe_hndlr
31: 0 0 0 0 LSAPIC cmc_hndlr
34: 19 0 0 0 IO-SAPIC-edge ide0
39: 0 0 0 0 IO-SAPIC-level acpi
48: 382 0 0 0 IO-SAPIC-level eth0
49: 0 2835 0 0 IO-SAPIC-level ioc0
50: 0 0 29 0 IO-SAPIC-level ioc1
51: 0 0 0 0 IO-SAPIC-level uhci_hcd:usb1
52: 51 0 0 0 IO-SAPIC-level uhci_hcd:usb2
232: 0 0 0 0 LSAPIC mca_rdzv
238: 0 0 0 0 LSAPIC perfmon
239: 122785 128748 129310 130523 LSAPIC timer
240: 0 0 0 0 LSAPIC mca_wkup
254: 29 77 101 102 LSAPIC IPI
ERR: 0

> It looks like the device
> being suspended never went through pci_device_enable() (e.g. ethernet
> interface wasn't up). It's harmless right now.

Everything's up. Perhaps we're trying to disable devices more than once?

2005-04-30 13:43:21

by Pavel Machek

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

Hi!

> > It looks like the device
> > being suspended never went through pci_device_enable() (e.g. ethernet
> > interface wasn't up). It's harmless right now.
>
> Everything's up. Perhaps we're trying to disable devices more than once?

Maybe userspace ifconfigs eth0 down, then we device_suspend it?
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms

2005-05-01 19:09:24

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

Hi,

Andrew Morton wrote:
> Pavel Machek <[email protected]> wrote:
>
>>
>>Without this patch, Linux provokes emergency disk shutdowns and
>>similar nastiness. It was in SuSE kernels for some time, IIRC.
>>
>
>
> With this patch when running `halt -p' my ia64 Tiger (using
> tiger_defconfig) gets a stream of badnesses in iosapic_unregister_intr()
> and then hangs up.
>
> Unfortunately it all seems to happen after the serial port has been
> disabled because nothing comes out. I set the console to a squitty font
> and took a piccy. See
> http://www.zip.com.au/~akpm/linux/patches/stuff/dsc02505.jpg
>
> I guess it's an ia64 problem. I'll leave the patch in -mm for now.
>

I guess the stream of badness was occured as follows:

pcibios_disable_device() for ia64 assumes that pci_enable_device()
and pci_disable_device() are balanced. But with 'properly stop
devices before power off' patch, pci_disable_device() becomes to be
called twice for e1000 device at halt time, through reboot_notifier_list
callback and through device_suspend(). As a result, iosapic_unregister_intr()
was called for already unregistered gsi and then stream of badness
was displayed.

I think the following patch will remove this stream of badness. I'm
sorry but I have not checked if the stream of badness is actually
removed because I'm on vacation and I can't look at my display
(I'm working via remote console). Could you try this patch?

By the way, I don't think this stream of badness is related to hang up,
because the problem (hang up) was reproduced even on my test kernel that
doesn't call pcibios_disable_device().

Thanks,
Kenji Kaneshige
---


There might be some cases that pci_disable_device() is called even if
the device is already disabled. In this case, pcibios_disable_device()
should not call acpi_pci_irq_disable() for the device.

Signed-off-by: Kenji Kaneshige <[email protected]>


---

linux-2.6.12-rc3-mm2-kanesige/arch/ia64/pci/pci.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletion(-)

diff -puN arch/ia64/pci/pci.c~fix_pcibios_disable_device_ia64 arch/ia64/pci/pci.c
--- linux-2.6.12-rc3-mm2/arch/ia64/pci/pci.c~fix_pcibios_disable_device_ia64 2005-05-02 03:12:23.000000000 +0900
+++ linux-2.6.12-rc3-mm2-kanesige/arch/ia64/pci/pci.c 2005-05-02 03:12:23.000000000 +0900
@@ -512,7 +512,8 @@ pcibios_enable_device (struct pci_dev *d
void
pcibios_disable_device (struct pci_dev *dev)
{
- acpi_pci_irq_disable(dev);
+ if (dev->is_enabled)
+ acpi_pci_irq_disable(dev);
}
#endif /* CONFIG_ACPI_DEALLOCATE_IRQ */


_

2005-05-01 19:58:24

by Pavel Machek

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

Hi!

> >>Without this patch, Linux provokes emergency disk shutdowns and
> >>similar nastiness. It was in SuSE kernels for some time, IIRC.
> >>
> >
> >
> >With this patch when running `halt -p' my ia64 Tiger (using
> >tiger_defconfig) gets a stream of badnesses in iosapic_unregister_intr()
> >and then hangs up.
> >
> >Unfortunately it all seems to happen after the serial port has been
> >disabled because nothing comes out. I set the console to a squitty font
> >and took a piccy. See
> >http://www.zip.com.au/~akpm/linux/patches/stuff/dsc02505.jpg
> >
> >I guess it's an ia64 problem. I'll leave the patch in -mm for now.
> >
>
> I guess the stream of badness was occured as follows:
>
> pcibios_disable_device() for ia64 assumes that pci_enable_device()
> and pci_disable_device() are balanced. But with 'properly stop
> devices before power off' patch, pci_disable_device() becomes to be
> called twice for e1000 device at halt time, through reboot_notifier_list
> callback and through device_suspend(). As a result,
> iosapic_unregister_intr()
> was called for already unregistered gsi and then stream of badness
> was displayed.
>
> I think the following patch will remove this stream of badness. I'm
> sorry but I have not checked if the stream of badness is actually
> removed because I'm on vacation and I can't look at my display
> (I'm working via remote console). Could you try this patch?
>
> By the way, I don't think this stream of badness is related to hang up,
> because the problem (hang up) was reproduced even on my test kernel that
> doesn't call pcibios_disable_device().

Looks good to me. Plus I guess we should remove reboot notifier from
e1000... It should be obsoleted by driver model.
Pavel

--
Boycott Kodak -- for their patent abuse against Java.

2005-05-01 22:21:28

by Adam Belay

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

On Mon, May 02, 2005 at 04:09:08AM +0900, Kenji Kaneshige wrote:
> Hi,
>
> Andrew Morton wrote:
> >Pavel Machek <[email protected]> wrote:
> >
> >>
> >>Without this patch, Linux provokes emergency disk shutdowns and
> >>similar nastiness. It was in SuSE kernels for some time, IIRC.
> >>
> >
> >
> >With this patch when running `halt -p' my ia64 Tiger (using
> >tiger_defconfig) gets a stream of badnesses in iosapic_unregister_intr()
> >and then hangs up.
> >
> >Unfortunately it all seems to happen after the serial port has been
> >disabled because nothing comes out. I set the console to a squitty font
> >and took a piccy. See
> >http://www.zip.com.au/~akpm/linux/patches/stuff/dsc02505.jpg
> >
> >I guess it's an ia64 problem. I'll leave the patch in -mm for now.
> >
>
> I guess the stream of badness was occured as follows:
>
> pcibios_disable_device() for ia64 assumes that pci_enable_device()
> and pci_disable_device() are balanced. But with 'properly stop
> devices before power off' patch, pci_disable_device() becomes to be
> called twice for e1000 device at halt time, through reboot_notifier_list
> callback and through device_suspend(). As a result,
> iosapic_unregister_intr()
> was called for already unregistered gsi and then stream of badness
> was displayed.
>
> I think the following patch will remove this stream of badness. I'm
> sorry but I have not checked if the stream of badness is actually
> removed because I'm on vacation and I can't look at my display
> (I'm working via remote console). Could you try this patch?
>
> By the way, I don't think this stream of badness is related to hang up,
> because the problem (hang up) was reproduced even on my test kernel that
> doesn't call pcibios_disable_device().
>
> Thanks,
> Kenji Kaneshige
> ---
>
>
> There might be some cases that pci_disable_device() is called even if
> the device is already disabled. In this case, pcibios_disable_device()
> should not call acpi_pci_irq_disable() for the device.
>
> Signed-off-by: Kenji Kaneshige <[email protected]>
>

Although this would solve the problem, it may or may not be the right thing to
do. The bug is not in pci_disable_device(), it's in the fact that we're
calling pci_disable_device() twice. Whether pci_disable_device() should ignore
this or create an error is an implementation decision. It might make sense to
have it print a warning. Greg, what are your thoughts?

What's important is that we don't want to suspend the device twice (in this
case suspend and reboot_notifier).

Thanks,
Adam

2005-05-01 22:28:48

by Adam Belay

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

On Thu, Apr 21, 2005 at 01:13:46PM +0200, Pavel Machek wrote:
>
> Without this patch, Linux provokes emergency disk shutdowns and
> similar nastiness. It was in SuSE kernels for some time, IIRC.
>
> Signed-off-by: Pavel Machek <[email protected]>

Hi Pavel,

Although I like using pm_message_t, I'm not sure if we want to commit to only
PMSG_SUSPEND and PMSG_FREEZE for shutdown and reboot. Would it be possible
to create a PMSG_HALT and PMSG_REBOOT? I think this would give drivers more
control and flexability to make the right decision. What is your opinion?

Of course, I'm still considering the posibility that we really want to do
PMSG_SUSPEND on a shutdown. This may work ok on X86, I'm not sure about other
architectures.

I know you mentioned previously adding more flags and data to pm_message_t,
what exactly are your plans?

Thanks,
Adam

2005-05-01 23:16:56

by Pavel Machek

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

Hi!

> > Without this patch, Linux provokes emergency disk shutdowns and
> > similar nastiness. It was in SuSE kernels for some time, IIRC.
> >
> > Signed-off-by: Pavel Machek <[email protected]>
>
> Hi Pavel,
>
> Although I like using pm_message_t, I'm not sure if we want to commit to only
> PMSG_SUSPEND and PMSG_FREEZE for shutdown and reboot. Would it be possible
> to create a PMSG_HALT and PMSG_REBOOT? I think this would give drivers more
> control and flexability to make the right decision. What is your opinion?
>
> Of course, I'm still considering the posibility that we really want to do
> PMSG_SUSPEND on a shutdown. This may work ok on X86, I'm not sure about other
> architectures.

Thats okay, nobody really knows yet. I believe that SUSPEND and HALT
are very similar, and flags best way to separate them. I believe that
FREEZE and REBOOT are very similar, too, and again would use flags to
tell between them.

> I know you mentioned previously adding more flags and data to pm_message_t,
> what exactly are your plans?

First I want type checking for pm_message_t. That's 2.6.12-early
material. Then, when it is *really clear* that flags are needed, I'll
add them. "really needed" as in "we have a driver where it matters".
Pavel
--
Boycott Kodak -- for their patent abuse against Java.

2005-05-02 00:06:36

by Adam Belay

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

On Mon, May 02, 2005 at 01:16:35AM +0200, Pavel Machek wrote:
> Hi!
>
> > > Without this patch, Linux provokes emergency disk shutdowns and
> > > similar nastiness. It was in SuSE kernels for some time, IIRC.
> > >
> > > Signed-off-by: Pavel Machek <[email protected]>
> >
> > Hi Pavel,
> >
> > Although I like using pm_message_t, I'm not sure if we want to commit to only
> > PMSG_SUSPEND and PMSG_FREEZE for shutdown and reboot. Would it be possible
> > to create a PMSG_HALT and PMSG_REBOOT? I think this would give drivers more
> > control and flexability to make the right decision. What is your opinion?
> >
> > Of course, I'm still considering the posibility that we really want to do
> > PMSG_SUSPEND on a shutdown. This may work ok on X86, I'm not sure about other
> > architectures.
>
> Thats okay, nobody really knows yet. I believe that SUSPEND and HALT
> are very similar, and flags best way to separate them. I believe that
> FREEZE and REBOOT are very similar, too, and again would use flags to
> tell between them.

I'm not so sure about SUSPEND and HALT being similar. It might be much faster
to have a routine that ignores slow state changes and goes directly for
stopping device activity. Still, I'm not entirely decided. On ACPI systems
SUSPEND should generally work, as it's the intended behavior for S4 which is
basically like S5 in many cases. However, having a specific HALT message
might allow driver developers to clarify this ambiguity on a per-device basis.

As for FREEZE, it does seem to match with REBOOT well. But it really depends
on what other things we intend to use FREEZE for (ex. CPUFREQ might require
something slightly different).

>
> > I know you mentioned previously adding more flags and data to pm_message_t,
> > what exactly are your plans?
>
> First I want type checking for pm_message_t. That's 2.6.12-early
> material. Then, when it is *really clear* that flags are needed, I'll
> add them. "really needed" as in "we have a driver where it matters".

I think it's already clear that we need to pass the specific device state.
Also the intended global system state transition might be helpful.

However, at the moment I'm considering using a slightly different API for
these sort of things. It would include "->prepare_state_change()" and
"->complete_state_change()" I'll have further justification for these
changes soon, however, I would like to leave the current stuff around also
as it would still be useful for shutdown and reboot, non-PM suspend issues,
and backward compatibilty.

Thanks,
Adam

2005-05-02 09:55:36

by Pavel Machek

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

Hi!

> > Thats okay, nobody really knows yet. I believe that SUSPEND and HALT
> > are very similar, and flags best way to separate them. I believe that
> > FREEZE and REBOOT are very similar, too, and again would use flags to
> > tell between them.
>
> I'm not so sure about SUSPEND and HALT being similar. It might be much faster
> to have a routine that ignores slow state changes and goes directly for
> stopping device activity. Still, I'm not entirely decided. On ACPI systems
> SUSPEND should generally work, as it's the intended behavior for S4 which is
> basically like S5 in many cases. However, having a specific HALT message
> might allow driver developers to clarify this ambiguity on a per-device basis.

Umm, you are right, HALT is somewhere between FREEZE and SUSPEND.

> > First I want type checking for pm_message_t. That's 2.6.12-early
> > material. Then, when it is *really clear* that flags are needed, I'll
> > add them. "really needed" as in "we have a driver where it matters".
>
> I think it's already clear that we need to pass the specific device state.
> Also the intended global system state transition might be helpful.

Global system state transition should be clear from flags. Specific
device state is needed for selective suspend, elsewhere I'd go for
helper function.

> However, at the moment I'm considering using a slightly different API for
> these sort of things. It would include "->prepare_state_change()" and
> "->complete_state_change()" I'll have further justification for these
> changes soon, however, I would like to leave the current stuff around also
> as it would still be useful for shutdown and reboot, non-PM suspend issues,
> and backward compatibilty.

I'd prefer not to have more callbacks (unless absolutely
neccessary). We used to have them, and people got it wrong....

Pavel
--
Boycott Kodak -- for their patent abuse against Java.

2005-05-16 08:40:53

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

Adam Belay <[email protected]> wrote:
>
> On Mon, May 02, 2005 at 04:09:08AM +0900, Kenji Kaneshige wrote:
> > Hi,
> >
> > Andrew Morton wrote:
> > >Pavel Machek <[email protected]> wrote:
> > >
> > >>
> > >>Without this patch, Linux provokes emergency disk shutdowns and
> > >>similar nastiness. It was in SuSE kernels for some time, IIRC.
> > >>
> > >
> > >
> > >With this patch when running `halt -p' my ia64 Tiger (using
> > >tiger_defconfig) gets a stream of badnesses in iosapic_unregister_intr()
> > >and then hangs up.

A little reminder that this bug remains unfixed...

> > >Unfortunately it all seems to happen after the serial port has been
> > >disabled because nothing comes out. I set the console to a squitty font
> > >and took a piccy. See
> > >http://www.zip.com.au/~akpm/linux/patches/stuff/dsc02505.jpg
> > >
> > >I guess it's an ia64 problem. I'll leave the patch in -mm for now.
> > >
> >
> > I guess the stream of badness was occured as follows:
> >
> > pcibios_disable_device() for ia64 assumes that pci_enable_device()
> > and pci_disable_device() are balanced. But with 'properly stop
> > devices before power off' patch, pci_disable_device() becomes to be
> > called twice for e1000 device at halt time, through reboot_notifier_list
> > callback and through device_suspend(). As a result,
> > iosapic_unregister_intr()
> > was called for already unregistered gsi and then stream of badness
> > was displayed.
> >
> > I think the following patch will remove this stream of badness. I'm
> > sorry but I have not checked if the stream of badness is actually
> > removed because I'm on vacation and I can't look at my display
> > (I'm working via remote console). Could you try this patch?
> >
> > By the way, I don't think this stream of badness is related to hang up,
> > because the problem (hang up) was reproduced even on my test kernel that
> > doesn't call pcibios_disable_device().
> >
> > Thanks,
> > Kenji Kaneshige
> > ---
> >
> >
> > There might be some cases that pci_disable_device() is called even if
> > the device is already disabled. In this case, pcibios_disable_device()
> > should not call acpi_pci_irq_disable() for the device.
> >
> > Signed-off-by: Kenji Kaneshige <[email protected]>
> >
>
> Although this would solve the problem, it may or may not be the right thing to
> do. The bug is not in pci_disable_device(), it's in the fact that we're
> calling pci_disable_device() twice. Whether pci_disable_device() should ignore
> this or create an error is an implementation decision. It might make sense to
> have it print a warning. Greg, what are your thoughts?
>
> What's important is that we don't want to suspend the device twice (in this
> case suspend and reboot_notifier).
>
> Thanks,
> Adam
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2005-05-19 12:44:29

by Kenji Kaneshige

[permalink] [raw]
Subject: Re: [patch] properly stop devices before poweroff

Hi,

I don't think the problem (hang up) is ia64 specific because
it is reproduced on my i386 box. The problem (hang up) seems
to occur on the machine that has Fusion MPT SCSI host adapter.
'SYNCHRONIZE_CACHE' command to MPT SCSI seems to never return
at device_shutdown() time.

Thanks,
Kenji Kaneshige


Andrew Morton wrote:
> Adam Belay <[email protected]> wrote:
>
>>On Mon, May 02, 2005 at 04:09:08AM +0900, Kenji Kaneshige wrote:
>>
>>>Hi,
>>>
>>>Andrew Morton wrote:
>>>
>>>>Pavel Machek <[email protected]> wrote:
>>>>
>>>>
>>>>>Without this patch, Linux provokes emergency disk shutdowns and
>>>>>similar nastiness. It was in SuSE kernels for some time, IIRC.
>>>>>
>>>>
>>>>
>>>>With this patch when running `halt -p' my ia64 Tiger (using
>>>>tiger_defconfig) gets a stream of badnesses in iosapic_unregister_intr()
>>>>and then hangs up.
>
>
> A little reminder that this bug remains unfixed...
>
>
>>>>Unfortunately it all seems to happen after the serial port has been
>>>>disabled because nothing comes out. I set the console to a squitty font
>>>>and took a piccy. See
>>>>http://www.zip.com.au/~akpm/linux/patches/stuff/dsc02505.jpg
>>>>
>>>>I guess it's an ia64 problem. I'll leave the patch in -mm for now.
>>>>
>>>
>>>I guess the stream of badness was occured as follows:
>>>
>>> pcibios_disable_device() for ia64 assumes that pci_enable_device()
>>> and pci_disable_device() are balanced. But with 'properly stop
>>> devices before power off' patch, pci_disable_device() becomes to be
>>> called twice for e1000 device at halt time, through reboot_notifier_list
>>> callback and through device_suspend(). As a result,
>>> iosapic_unregister_intr()
>>> was called for already unregistered gsi and then stream of badness
>>> was displayed.
>>>
>>>I think the following patch will remove this stream of badness. I'm
>>>sorry but I have not checked if the stream of badness is actually
>>>removed because I'm on vacation and I can't look at my display
>>>(I'm working via remote console). Could you try this patch?
>>>
>>>By the way, I don't think this stream of badness is related to hang up,
>>>because the problem (hang up) was reproduced even on my test kernel that
>>>doesn't call pcibios_disable_device().
>>>
>>>Thanks,
>>>Kenji Kaneshige
>>>---
>>>
>>>
>>>There might be some cases that pci_disable_device() is called even if
>>>the device is already disabled. In this case, pcibios_disable_device()
>>>should not call acpi_pci_irq_disable() for the device.
>>>
>>>Signed-off-by: Kenji Kaneshige <[email protected]>
>>>
>>Although this would solve the problem, it may or may not be the right thing to
>>do. The bug is not in pci_disable_device(), it's in the fact that we're
>>calling pci_disable_device() twice. Whether pci_disable_device() should ignore
>>this or create an error is an implementation decision. It might make sense to
>>have it print a warning. Greg, what are your thoughts?
>>
>>What's important is that we don't want to suspend the device twice (in this
>>case suspend and reboot_notifier).
>>
>>Thanks,
>>Adam
>>-
>>To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
>>the body of a message to [email protected]
>>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>