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
|
Home