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] pvops: fix "xm save -c" issue

Hi Ian,

I split the patch into pm infrastructure part and xen part.
Actually, I've never post to linux community, so I'm not sure what to
do.

Thanks,
KAZ

Signed-off-by: Kenji Wakamiya <wkenji@xxxxxxxxxxxxxx>
Signed-off-by: Kazuhiro Suzuki <kaz@xxxxxxxxxxxxxx>


From: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] pvops: fix "xm save -c" issue
Date: Fri, 21 Jan 2011 09:14:30 +0000

> On Fri, 2011-01-21 at 06:28 +0000, SUZUKI, Kazuhiro wrote:
>> Hi,
>> 
>> The following patch fixes 'xm save -c' issue.
>> We defined 'PMSG_CANCEL' message for suspend cancel situation and
>> suspend_cancel handler in pm_ops struct.
>> If the suspend_cancel is defined, suspend_cancel() is called instead
>> of resume().
> 
> Thanks. I like the general shape of this patch but the core pm
> infrastructure change needs to be split out and sent upstream via the
> power management maintainer. I think this is Rafael J. Wysocki
> <rjw@xxxxxxx> and the linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx list.
> 
> If you also split out the Xen conversion to dev_pm_ops, without the
> suspend_cancel bit, then I think we can take that bit now and then you
> can resend the bit which adds the suspend_cancel bits once the core
> stuff is upstream.
> 
> Ian.
> 
>> 
>> Thanks,
>> KAZ
>> 
>> Signed-off-by: Kenji Wakamiya <wkenji@xxxxxxxxxxxxxx>
>> Signed-off-by: Kazuhiro Suzuki <kaz@xxxxxxxxxxxxxx>
>> 
>> 
>> From: Kenji Wakamiya <wkenji@xxxxxxxxxxxxxx>
>> Subject: Re: [Xen-devel] [PATCH] pvops: fix "xm save -c" issue
>> Date: Fri, 21 Jan 2011 14:35:09 +0900
>> 
>> > Hi Konrand, and sorry for very late response.
>> > 
>> > (2011/01/11 2:01), Konrad Rzeszutek Wilk wrote:
>> >>> With this change how is the effect of dpm_suspend_start undone in the
>> >>> suspend cancelled case?
>> >>>
>> >>> Currently we have
>> >>>  dpm_suspend_start(PMSG_SUSPEND)
>> >>>   xs_suspend
>> >>>     dpm_suspend_noirq(PMSG_SUSPEND)
>> >>>        SUSPEND
>> >>>     dpm_resume_noirq(PMSG_RESUME)
>> >>>   xs_resume or xs_supend_cancel
>> >>>  dpm_resume_end(PMSG_RESUME)
>> >>>
>> >>> Which seems nicely nested and logical but by only calling dpm_resume_end
>> >>> in the non-cancelled case we seem to be unbalancing things.
>> >>>
>> >>> Do we need some sort of dpm_resume_cancel, or some way of pushing the
>> >>> cancelled flag down into the individual xenbus_device.resume handlers?
>> >>>
>> >>> Should we maybe simply be using a difference PMSG_XXX in the cancelled
>> >>> case? Is this what one of PMSG_RESTORE or PMSG_RECOVER means?
>> >>>
>> >>> Looks like to propagate the PMSG_* to the actual device resume functions
>> >>> we would need to provide a pm_ops for the struct bus xenbus_frontend
>> >>> instead of relying on the legacy handlers. This is probably a
>> >>> independently good idea anyway.
>> >>
>> >> ping?
>> >>
>> >> Kenji any ideas or patches to address Ian's comments?
>> > 
>> > My colleague made a patch which reflected Ian's comments, so I will ask 
>> > him to post it. Please wait a little.
>> > 
>> > Thanks,
>> > Kenji
>> > 
>> > 
>> > _______________________________________________
>> > Xen-devel mailing list
>> > Xen-devel@xxxxxxxxxxxxxxxxxxx
>> > http://lists.xensource.com/xen-devel
> 
> 
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 8aa2443..b743e8f 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -181,6 +181,13 @@ static int pm_op(struct device *dev,
                        suspend_report_result(ops->suspend, error);
                }
                break;
+       case PM_EVENT_CANCEL:
+               if (ops->suspend_cancel) {
+                       error = ops->suspend_cancel(dev);
+                       suspend_report_result(ops->suspend_cancel, error);
+                       break;
+               }
+               /* Fall through */
        case PM_EVENT_RESUME:
                if (ops->resume) {
                        error = ops->resume(dev);
@@ -291,6 +298,8 @@ static char *pm_verb(int event)
        switch (event) {
        case PM_EVENT_SUSPEND:
                return "suspend";
+       case PM_EVENT_CANCEL:
+               return "cancel";
        case PM_EVENT_RESUME:
                return "resume";
        case PM_EVENT_FREEZE:
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 3b7e04b..aa89839 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -197,6 +197,7 @@ struct dev_pm_ops {
        int (*prepare)(struct device *dev);
        void (*complete)(struct device *dev);
        int (*suspend)(struct device *dev);
+       int (*suspend_cancel)(struct device *dev);
        int (*resume)(struct device *dev);
        int (*freeze)(struct device *dev);
        int (*thaw)(struct device *dev);
@@ -291,6 +292,7 @@ struct dev_pm_ops name = { \
 #define PM_EVENT_USER          0x0100
 #define PM_EVENT_REMOTE                0x0200
 #define PM_EVENT_AUTO          0x0400
+#define PM_EVENT_CANCEL                0x0800
 
 #define PM_EVENT_SLEEP         (PM_EVENT_SUSPEND | PM_EVENT_HIBERNATE)
 #define PM_EVENT_USER_SUSPEND  (PM_EVENT_USER | PM_EVENT_SUSPEND)
@@ -308,6 +310,7 @@ struct dev_pm_ops name = { \
 #define PMSG_THAW      ((struct pm_message){ .event = PM_EVENT_THAW, })
 #define PMSG_RESTORE   ((struct pm_message){ .event = PM_EVENT_RESTORE, })
 #define PMSG_RECOVER   ((struct pm_message){ .event = PM_EVENT_RECOVER, })
+#define PMSG_CANCEL    ((struct pm_message){ .event = PM_EVENT_CANCEL, })
 #define PMSG_USER_SUSPEND      ((struct pm_message) \
                                        { .event = PM_EVENT_USER_SUSPEND, })
 #define PMSG_USER_RESUME       ((struct pm_message) \
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 3f71199..22c6288 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1293,7 +1293,7 @@ static void xennet_disconnect_backend(struct 
netfront_info *info)
        info->rx.sring = NULL;
 }
 
-static int netfront_suspend(struct xenbus_device *dev, pm_message_t state)
+static int netfront_suspend(struct xenbus_device *dev)
 {
        struct netfront_info *info = dev_get_drvdata(&dev->dev);
        struct hrtimer *timer = &info->smart_poll.timer;
diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index 0b50906..845afb8 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -148,10 +148,11 @@ out_resume:
        if (!cancelled) {
                xen_arch_resume();
                xs_resume();
-       } else
+               dpm_resume_end(PMSG_RESUME);
+       } else {
                xs_suspend_cancel();
-
-       dpm_resume_end(PMSG_RESUME);
+               dpm_resume_end(PMSG_CANCEL);
+       }
 
        /* Make sure timer events get retriggered on all CPUs */
        clock_was_set();
diff --git a/drivers/xen/xenbus/xenbus_probe.c 
b/drivers/xen/xenbus/xenbus_probe.c
index 3a83ba2..019337a 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -575,7 +575,7 @@ void xenbus_dev_changed(const char *node, struct 
xen_bus_type *bus)
 }
 EXPORT_SYMBOL_GPL(xenbus_dev_changed);
 
-int xenbus_dev_suspend(struct device *dev, pm_message_t state)
+int xenbus_dev_suspend(struct device *dev)
 {
        int err = 0;
        struct xenbus_driver *drv;
@@ -587,7 +587,7 @@ int xenbus_dev_suspend(struct device *dev, pm_message_t 
state)
                return 0;
        drv = to_xenbus_driver(dev->driver);
        if (drv->suspend)
-               err = drv->suspend(xdev, state);
+               err = drv->suspend(xdev);
        if (err)
                printk(KERN_WARNING
                       "xenbus: suspend %s failed: %i\n", dev_name(dev), err);
@@ -595,6 +595,14 @@ int xenbus_dev_suspend(struct device *dev, pm_message_t 
state)
 }
 EXPORT_SYMBOL_GPL(xenbus_dev_suspend);
 
+int xenbus_dev_suspend_cancel(struct device *dev)
+{
+       /* Do nothing */
+       DPRINTK("cancel");
+       return 0;
+}
+EXPORT_SYMBOL_GPL(xenbus_dev_suspend_cancel);
+
 int xenbus_dev_resume(struct device *dev)
 {
        int err;
diff --git a/drivers/xen/xenbus/xenbus_probe.h 
b/drivers/xen/xenbus/xenbus_probe.h
index 0e5fc4c..bf4a793 100644
--- a/drivers/xen/xenbus/xenbus_probe.h
+++ b/drivers/xen/xenbus/xenbus_probe.h
@@ -62,7 +62,8 @@ extern void xenbus_dev_changed(const char *node, struct 
xen_bus_type *bus);
 
 extern void xenbus_dev_shutdown(struct device *_dev);
 
-extern int xenbus_dev_suspend(struct device *dev, pm_message_t state);
+extern int xenbus_dev_suspend(struct device *dev);
+extern int xenbus_dev_suspend_cancel(struct device *dev);
 extern int xenbus_dev_resume(struct device *dev);
 
 extern void xenbus_otherend_changed(struct xenbus_watch *watch,
diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c 
b/drivers/xen/xenbus/xenbus_probe_frontend.c
index 5413248..928fc10 100644
--- a/drivers/xen/xenbus/xenbus_probe_frontend.c
+++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
@@ -82,6 +82,11 @@ static struct device_attribute xenbus_frontend_dev_attrs[] = 
{
        __ATTR_NULL
 };
 
+static struct dev_pm_ops xenbus_pm_ops = {
+       .suspend        = xenbus_dev_suspend,
+       .suspend_cancel = xenbus_dev_suspend_cancel,
+       .resume         = xenbus_dev_resume,
+};
 
 static struct xen_bus_type xenbus_frontend = {
        .root = "device",
@@ -98,8 +103,7 @@ static struct xen_bus_type xenbus_frontend = {
                .shutdown = xenbus_dev_shutdown,
                .dev_attrs= xenbus_frontend_dev_attrs,
 
-               .suspend  = xenbus_dev_suspend,
-               .resume   = xenbus_dev_resume,
+               .pm       = &xenbus_pm_ops,
        },
 };
 
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index 542ca7c..23e7f25 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -91,7 +91,7 @@ struct xenbus_driver {
        void (*otherend_changed)(struct xenbus_device *dev,
                                 enum xenbus_state backend_state);
        int (*remove)(struct xenbus_device *dev);
-       int (*suspend)(struct xenbus_device *dev, pm_message_t state);
+       int (*suspend)(struct xenbus_device *dev);
        int (*resume)(struct xenbus_device *dev);
        int (*uevent)(struct xenbus_device *, struct kobj_uevent_env *);
        struct device_driver driver;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel