Messages in this thread Patch in this message | | | Date | Sun, 1 Aug 2004 22:42:11 -0700 (PDT) | From | Patrick Mochel <> | Subject | Re: [0/25] Merge pmdisk and swsusp |
| |
On Tue, 20 Jul 2004, Pavel Machek wrote:
> Followup patch: > > * if machine halt fails, it is very dangerous to continue. > > diff -ur linux.middle/kernel/power/disk.c linux/kernel/power/disk.c > --- linux.middle/kernel/power/disk.c 2004-07-19 08:58:08.000000000 -0700 > +++ linux/kernel/power/disk.c 2004-07-19 15:00:16.000000000 -0700 > @@ -63,6 +63,9 @@ > break; > } > machine_halt(); > + /* Valid image is on the disk, if we continue we risk serious data corruption > + after resume. */ > + while(1); > device_power_up(); > local_irq_restore(flags); > return 0;
This is nasty. We have to fail gracefully, ideally without expecting user input.
Adding 'while(1)' will cause the CPU to enter a busy loop, artificially increasing the power consumption of the system, which would be counter- productive in a system that was configured to suspend when the battery was low.
We need to at least print a message specifying what happened and instructing them to reboot. It's dorky, but over time, all every system should eventually be fixed to either enter a low-power mode or shut down properly.
Perhaps we could also fill in machine_halt(), which the patch below also does.
> * software_suspend() did not check for smp, this fixes it.
Applied, thanks.
> * copy_page() is dangerous. This is actually my fault.
Why is copy_page() dangerous? Shouldn't it be fixed if that is the case?
Thanks,
Pat ===== arch/i386/kernel/reboot.c 1.16 vs edited ===== --- 1.16/arch/i386/kernel/reboot.c 2004-07-05 03:28:50 -07:00 +++ edited/arch/i386/kernel/reboot.c 2004-08-01 22:40:58 -07:00 @@ -367,6 +367,8 @@
void machine_halt(void) { + while (1) + asm volatile ("hlt":::"memory"); }
EXPORT_SYMBOL(machine_halt); ===== kernel/power/disk.c 1.16 vs edited ===== --- 1.16/kernel/power/disk.c 2004-08-01 20:36:39 -07:00 +++ edited/kernel/power/disk.c 2004-08-01 22:38:19 -07:00 @@ -59,6 +59,7 @@ machine_restart(NULL); break; } + printk(KERN_EMERG "Suspend-to-disk succeeded, but power-down failed. Please reboot.\n"); machine_halt(); device_power_up(); local_irq_restore(flags); - 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/
| |