Messages in this thread | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH 3/3 -mm] kexec based hibernation -v6: kexec hibernate/resume | Date | Tue, 20 Nov 2007 03:24:30 +0100 |
| |
On Tuesday, 20 of November 2007, Huang, Ying wrote: > On Mon, 2007-11-19 at 19:22 +0100, Rafael J. Wysocki wrote: > > > +#ifdef CONFIG_KEXEC > > > +static void kexec_hibernate_power_down(void) > > > +{ > > > + switch (hibernation_mode) { > > > + case HIBERNATION_TEST: > > > + case HIBERNATION_TESTPROC: > > > + break; > > > + case HIBERNATION_REBOOT: > > > + machine_restart(NULL); > > > + break; > > > + case HIBERNATION_PLATFORM: > > > + if (!hibernation_ops) > > > + break; > > > + hibernation_ops->enter(); > > > > hibernation_platform_enter() should be used here (as of the current mainline). > > The power_down will be called with interrupt disabled, device suspended, > non-boot CPU disabled. But the latest hibernate_platform_enter calls the > device_suspend, disable_nonboot_cpus etc function. So, I use > hibernation_ops->enter() directly instead of > hibernation_platform_enter().
Hm, you need to call device_power_down(PMSG_SUSPEND) before hibernation_ops->enter().
Also, all of the ACPI global calls need to be carried out before that and the devices should be suspended rather than shut down in that case.
That's why hibernation_platform_enter() has been introduced, BTW.
> > > + /* We should never get here */ > > > + while (1); > > > + break; > > > + case HIBERNATION_SHUTDOWN: > > > + machine_power_off(); > > > + break; > > > + } > > > + machine_halt(); > > > + /* > > > + * Valid image is on the disk, if we continue we risk serious data > > > + * corruption after resume. > > > + */ > > > + printk(KERN_CRIT "Please power me down manually\n"); > > > + while (1); > > > +} > > > > Hm, what's the difference between the above function and power_down(), > > actually? > > Same as above.
I see, but I'm not sure it's correct.
> > > + > > > +int kexec_hibernate(struct kimage *image) > > > +{ > > > + int error; > > > + int platform_mode = (hibernation_mode == HIBERNATION_PLATFORM); > > > + unsigned long cmd_ret; > > > + > > > + mutex_lock(&pm_mutex); > > > + > > > + pm_prepare_console(); > > > + suspend_console(); > > > + > > > + error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE); > > > + if (error) > > > + goto Resume_console; > > > + > > > + error = platform_start(platform_mode); > > > + if (error) > > > + goto Resume_console; > > > + > > > + error = device_suspend(PMSG_FREEZE); > > > + if (error) > > > + goto Resume_console; > > > + > > > + error = platform_pre_snapshot(platform_mode); > > > + if (error) > > > + goto Resume_devices; > > > + > > > + error = disable_nonboot_cpus(); > > > + if (error) > > > + goto Resume_devices; > > > > I wonder if it's viable to merge the above with hibernate() and > > hibernation_snapshot() somehow, to avoid code duplication? > > Yes. Most code are duplicated. But there is one advantage not to merge > them: power_down can called with IRQ disabled to make it possible to > eliminate the freezer.
Please leave the freezer alone for now. We'll clean up all that when we decide to remove it.
> I think it is possible to merge the two implementation. I will try to do > it.
Yes, please use as much of the existing code as reasonably possible, so that we don't need to duplicate changes in the future.
> > Apart from the above, there's some new debug code to be added to disk.c > > in 2.6.25. It's in the ACPI test tree right now and you can get it as > > individual patches from: > > http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.24-rc2/patches/ > > (patches 10-12). > > > > Please base your changes on top of that. > > OK, I will do it.
Thanks.
There will be some more cosmetic changes in this area, probably before 2.6.25, so please stay tuned.
Greetings, Rafael - 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/
| |