lkml.org 
[lkml]   [2004]   [Aug]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [0/25] Merge pmdisk and swsusp
Hi!

> > * 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.

Ok, it was a "too hot hotfix". Your solution is better (but see below).

> Perhaps we could also fill in machine_halt(), which the patch below also
> does.

Good.

> > * copy_page() is dangerous. This is actually my fault.
>
> Why is copy_page() dangerous? Shouldn't it be fixed if that is the
> case?

copy_page sometimes changes struct task_struct, does copy, changes it
back. That makes it bad choice for copying task_structs,
unfortunately. Do you want me to retransmit the patch?

> ===== 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);

If i386 got it wrong, it is possible that other architectures get it
wrong, too. Fixing i386 is good, but we should not risk continuing
machine with valid image on disk.

I guess "device_power_up / local_irq_restore" should be replaced with
BUG() or while(1)?
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
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/

\
 
 \ /
  Last update: 2005-03-22 14:05    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site