WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] Use freeze/thaw/restore PM events for Guest susp

On Mon, 2011-02-14 at 21:47 +0000, Shriram Rajagopalan wrote:
> On Mon, Feb 14, 2011 at 11:06 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 
> wrote:
> > On Mon, 2011-02-14 at 16:21 +0000, Shriram Rajagopalan wrote:
> >> Use PM_FREEZE, PM_THAW and PM_RESTORE power events for
> >> suspend/resume functionality, instead of PM_SUSPEND and
> >> PM_RESUME. Use of these pm events also fixes the Xen Guest
> >> hangup when taking checkpoints. When a suspend event is cancelled
> >> (i.e. while taking checkpoints once/continuously), we use PM_THAW
> >> instead of PM_RESUME. PM_RESTORE is used when suspend is not
> >> cancelled. See Documentation/power/devices.txt and linux/pm.h
> >> for more info about freeze, thaw and restore. The sequence of
> >> pm events in a suspend-resume scenario is shown below.
> >>
> >>       dpm_suspend_start(PMSG_FREEZE);
> >>
> >>               dpm_suspend_noirq(PMSG_FREEZE);
> >>
> >>                        sysdev_suspend(PMSG_FREEZE);
> >>                        cancelled = suspend_hypercall()
> >>                        sysdev_resume();
> >>
> >>                dpm_resume_noirq(cancelled ? PMSG_THAW : PMSG_RESTORE);
> >>
> >>        dpm_resume_end(cancelled ? PMSG_THAW : PMSG_RESTORE);
> >
> > Thanks.
> >
> > Which tree/branch is this against?
> >
> Oh I thought that info be present in the git indices
> "index 5413248..ff32ffb 100644" .

I think those are the indices of the actual versions of the file, rather
than the tree->commit->branch which contains them. I'm sure you could
map up that chain but suspect it'd be a rather brute force affair.

> Its against my local git branch (which tracks xen/stable-2.6.32.x and
> is uptodate).

In general we'd prefer to get this sort of thing fixed in the mainline
kernel (e.g. Linus' tree) first and then consider backports to the
stable branch as necessary.

> Sorry, I am still a bit of git newbie.

No problem.

> > Can you please at least do the dev_pm_ops as a separate patch to 
> > allow bisectability etc. (generally each patch should be a single 
> > logical change so if the remaineder can be sensibly split too it is
> > worth doing so).

> ok

You can probably take Kazuhiro's first patch verbatim for this bit? If
you save the mail to a file you can apply it with "git am < foo.mail"
and git will preserve authorship attribution etc and this will persist
through "git format-patch" and "git send-email" etc.

> > Did you test regular save/restore as well as cancelled migrations? What
> > about PVHMV guests?
> >
> >> +static struct dev_pm_ops xenbus_pm_ops = {
> >> +     .suspend = xenbus_dev_suspend,
> >> +     .resume  = xenbus_dev_resume,
> >> +     .freeze  = xenbus_dev_suspend,
> >> +     .thaw    = xenbus_dev_cancel,
> >> +     .restore = xenbus_dev_resume,
> >> +};
> >
> > Perhaps xenbus_dev_thaw?
> >
> > Are suspend/freeze and resume/restore really the same?
> >
> Semantically they are not. From the documentation in pm.h,
[...]
> So, in our case, suspend/freeze and resume/restore are basically same.
> suspend-cancel is a thaw event.

OK.

> > Once we've transitioned to the PMSG_FREEZE way of doing things do we
> > still need to keep the other hooks around? If not then the other ones
> > could be renamed as well?
> >
> If your question is whether we can change
> static struct dev_pm_ops xenbus_pm_ops = {
>      .suspend = xenbus_dev_suspend,
>      .resume  = xenbus_dev_resume,
>      .freeze  = xenbus_dev_suspend,
>      .thaw    = xenbus_dev_cancel,
>      .restore = xenbus_dev_resume,
> };
> to just
> static struct dev_pm_ops xenbus_pm_ops = {
>      .freeze  = xenbus_dev_freeze,
>      .thaw    = xenbus_dev_thaw,
>      .restore = xenbus_dev_restore,
> };
> 
> then the answer is no, AFAICS, from the code in drivers/base/power/main.c
> (pm_op function).

Looking at pm_op it doesn't appear to be an error to not implement one
or more hooks. For example we currently don't implement freeze/thaw and
that is currently ok because we don't initiate that sort of suspend.

Currently drivers/xen/manage.c hangs the suspend code off
CONFIG_PM_SLEEP, I wonder if it shouldn't be CONFIG_SUSPEND already and
become CONFIG_HIBERNATE with your change?

I also wonder if shutdown_handler shouldn't check it is actually going
to be able to perform the suspend _before_ acknowledging the suspend
request, but that is outside the scope of this patch.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel