Shriram Rajagopalan
2011-Feb-15 23:10 UTC
[Xen-devel] [PATCH] xenbus PM events support (was: [PATCH 2/2] Fix hangup after creating checkpoint on Xen.)
From: SUZUKI, Kazuhiro <kaz@jp.fujitsu.com>
Hi,
This is a Xen part patch.
Thanks,
KAZ
Signed-off-by: Kenji Wakamiya <wkenji@jp.fujitsu.com>
Signed-off-by: Kazuhiro Suzuki <kaz@jp.fujitsu.com>
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
---
drivers/xen/xenbus/xenbus_probe.c | 12 ++++++++++--
drivers/xen/xenbus/xenbus_probe.h | 3 ++-
drivers/xen/xenbus/xenbus_probe_frontend.c | 9 +++++++--
include/xen/xenbus.h | 2 +-
4 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus_probe.c
b/drivers/xen/xenbus/xenbus_probe.c
index baa65e7..7397695 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -577,7 +577,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;
@@ -590,7 +590,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);
@@ -642,6 +642,14 @@ int xenbus_dev_resume(struct device *dev)
}
EXPORT_SYMBOL_GPL(xenbus_dev_resume);
+int xenbus_dev_cancel(struct device *dev)
+{
+ /* Do nothing */
+ DPRINTK("cancel");
+ return 0;
+}
+EXPORT_SYMBOL_GPL(xenbus_dev_cancel);
+
/* A flag to determine if xenstored is ''ready'' (i.e. has
started) */
int xenstored_ready = 0;
diff --git a/drivers/xen/xenbus/xenbus_probe.h
b/drivers/xen/xenbus/xenbus_probe.h
index 2466581..888b990 100644
--- a/drivers/xen/xenbus/xenbus_probe.h
+++ b/drivers/xen/xenbus/xenbus_probe.h
@@ -64,8 +64,9 @@ 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_resume(struct device *dev);
+extern int xenbus_dev_cancel(struct device *dev);
extern void xenbus_otherend_changed(struct xenbus_watch *watch,
const char **vec, unsigned int len,
diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c
b/drivers/xen/xenbus/xenbus_probe_frontend.c
index 5bcc2d6..9ad8868 100644
--- a/drivers/xen/xenbus/xenbus_probe_frontend.c
+++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
@@ -85,6 +85,12 @@ static struct device_attribute xenbus_frontend_dev_attrs[] =
{
__ATTR_NULL
};
+static struct dev_pm_ops xenbus_pm_ops = {
+ .suspend = xenbus_dev_suspend,
+ .resume = xenbus_dev_resume,
+ .thaw = xenbus_dev_cancel,
+};
+
static struct xen_bus_type xenbus_frontend = {
.root = "device",
.levels = 2, /* device/type/<id> */
@@ -100,8 +106,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 7a1d15f..5467369 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -92,7 +92,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;
--
1.7.0.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Shriram Rajagopalan
2011-Feb-15 23:20 UTC
Re: [Xen-devel] [PATCH] xenbus PM events support (was: [PATCH 2/2] Fix hangup after creating checkpoint on Xen.)
The above patch is against upstream/for-linus tree. The xen-netfront driver does not have any suspend handler at all, in this tree. I checked other trees (upstream/pvhvm, upstream/xen, upstream/netfront). Same issue. So, I dont know which tree Kazuhiro''s original patch applies to. Shriram On Tue, Feb 15, 2011 at 3:10 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:> From: SUZUKI, Kazuhiro <kaz@jp.fujitsu.com> > > Hi, > > This is a Xen part patch. > > Thanks, > KAZ > > Signed-off-by: Kenji Wakamiya <wkenji@jp.fujitsu.com> > Signed-off-by: Kazuhiro Suzuki <kaz@jp.fujitsu.com> > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> > --- > drivers/xen/xenbus/xenbus_probe.c | 12 ++++++++++-- > drivers/xen/xenbus/xenbus_probe.h | 3 ++- > drivers/xen/xenbus/xenbus_probe_frontend.c | 9 +++++++-- > include/xen/xenbus.h | 2 +- > 4 files changed, 20 insertions(+), 6 deletions(-) > > diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c > index baa65e7..7397695 100644 > --- a/drivers/xen/xenbus/xenbus_probe.c > +++ b/drivers/xen/xenbus/xenbus_probe.c > @@ -577,7 +577,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; > @@ -590,7 +590,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); > @@ -642,6 +642,14 @@ int xenbus_dev_resume(struct device *dev) > } > EXPORT_SYMBOL_GPL(xenbus_dev_resume); > > +int xenbus_dev_cancel(struct device *dev) > +{ > + /* Do nothing */ > + DPRINTK("cancel"); > + return 0; > +} > +EXPORT_SYMBOL_GPL(xenbus_dev_cancel); > + > /* A flag to determine if xenstored is ''ready'' (i.e. has started) */ > int xenstored_ready = 0; > > diff --git a/drivers/xen/xenbus/xenbus_probe.h b/drivers/xen/xenbus/xenbus_probe.h > index 2466581..888b990 100644 > --- a/drivers/xen/xenbus/xenbus_probe.h > +++ b/drivers/xen/xenbus/xenbus_probe.h > @@ -64,8 +64,9 @@ 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_resume(struct device *dev); > +extern int xenbus_dev_cancel(struct device *dev); > > extern void xenbus_otherend_changed(struct xenbus_watch *watch, > const char **vec, unsigned int len, > diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c > index 5bcc2d6..9ad8868 100644 > --- a/drivers/xen/xenbus/xenbus_probe_frontend.c > +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c > @@ -85,6 +85,12 @@ static struct device_attribute xenbus_frontend_dev_attrs[] = { > __ATTR_NULL > }; > > +static struct dev_pm_ops xenbus_pm_ops = { > + .suspend = xenbus_dev_suspend, > + .resume = xenbus_dev_resume, > + .thaw = xenbus_dev_cancel, > +}; > + > static struct xen_bus_type xenbus_frontend = { > .root = "device", > .levels = 2, /* device/type/<id> */ > @@ -100,8 +106,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 7a1d15f..5467369 100644 > --- a/include/xen/xenbus.h > +++ b/include/xen/xenbus.h > @@ -92,7 +92,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; > -- > 1.7.0.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Feb-16 09:43 UTC
Re: [Xen-devel] [PATCH] xenbus PM events support (was: [PATCH 2/2] Fix hangup after creating checkpoint on Xen.)
On Tue, 2011-02-15 at 23:20 +0000, Shriram Rajagopalan wrote:> The above patch is against upstream/for-linus tree.In general it is better to pick either a mainline kernel (e.g. the master branch from git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git), one of the stable-* or next-* branches from xen.git or a specific topic branch. The for-* branches are usually just throwaway branches which are not necessarily kept up to date or relevant standalone. (in this case I think it''s ok since it is just 2.6.38-rc1 + a little bit)> The xen-netfront driver does not have any suspend handler at all, in this tree. > I checked other trees (upstream/pvhvm, upstream/xen, > upstream/netfront). Same issue.This patch doesn''t appear to touch xen-netfront though? It looks like the netfront driver only needs to do work on resume, not suspend. This is consistent with the explanation of why freeze/suspend and thaw/resume are the same under Xen which you gave in an earlier mail, isn''t it? A quick "git grep xenbus_driver" doesn''t show any of the other frontends having a suspend callback either.> So, I dont know which tree Kazuhiro''s original patch applies to. > > Shriram > > On Tue, Feb 15, 2011 at 3:10 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote: > > From: SUZUKI, Kazuhiro <kaz@jp.fujitsu.com> > > > > Hi, > > > > This is a Xen part patch.Can you improve on this description please. I meant to respond to this in the initial posting and forgot once the thread got sidetracked into what the correct approach is. My advice to "just pipe the mail into git am" may have been a bit misleading, sorry. In general it''s fine to fixup the description and/or minor nits with the patch as it passes through your hand. If the changes are significant it''s useful to acknowledge what you''ve done, many people add a [shriram--improved commit message] style tag below their signed off. In this case the actual patch itself is fine and: Acked-by: Ian Campbell <ian.campbell@citrix.com> Ian.> > > > Thanks, > > KAZ > > > > Signed-off-by: Kenji Wakamiya <wkenji@jp.fujitsu.com> > > Signed-off-by: Kazuhiro Suzuki <kaz@jp.fujitsu.com> > > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> > > --- > > drivers/xen/xenbus/xenbus_probe.c | 12 ++++++++++-- > > drivers/xen/xenbus/xenbus_probe.h | 3 ++- > > drivers/xen/xenbus/xenbus_probe_frontend.c | 9 +++++++-- > > include/xen/xenbus.h | 2 +- > > 4 files changed, 20 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c > > index baa65e7..7397695 100644 > > --- a/drivers/xen/xenbus/xenbus_probe.c > > +++ b/drivers/xen/xenbus/xenbus_probe.c > > @@ -577,7 +577,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; > > @@ -590,7 +590,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); > > @@ -642,6 +642,14 @@ int xenbus_dev_resume(struct device *dev) > > } > > EXPORT_SYMBOL_GPL(xenbus_dev_resume); > > > > +int xenbus_dev_cancel(struct device *dev) > > +{ > > + /* Do nothing */ > > + DPRINTK("cancel"); > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(xenbus_dev_cancel); > > + > > /* A flag to determine if xenstored is ''ready'' (i.e. has started) */ > > int xenstored_ready = 0; > > > > diff --git a/drivers/xen/xenbus/xenbus_probe.h b/drivers/xen/xenbus/xenbus_probe.h > > index 2466581..888b990 100644 > > --- a/drivers/xen/xenbus/xenbus_probe.h > > +++ b/drivers/xen/xenbus/xenbus_probe.h > > @@ -64,8 +64,9 @@ 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_resume(struct device *dev); > > +extern int xenbus_dev_cancel(struct device *dev); > > > > extern void xenbus_otherend_changed(struct xenbus_watch *watch, > > const char **vec, unsigned int len, > > diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c > > index 5bcc2d6..9ad8868 100644 > > --- a/drivers/xen/xenbus/xenbus_probe_frontend.c > > +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c > > @@ -85,6 +85,12 @@ static struct device_attribute xenbus_frontend_dev_attrs[] = { > > __ATTR_NULL > > }; > > > > +static struct dev_pm_ops xenbus_pm_ops = { > > + .suspend = xenbus_dev_suspend, > > + .resume = xenbus_dev_resume, > > + .thaw = xenbus_dev_cancel, > > +}; > > + > > static struct xen_bus_type xenbus_frontend = { > > .root = "device", > > .levels = 2, /* device/type/<id> */ > > @@ -100,8 +106,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 7a1d15f..5467369 100644 > > --- a/include/xen/xenbus.h > > +++ b/include/xen/xenbus.h > > @@ -92,7 +92,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; > > -- > > 1.7.0.4 > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xensource.com > > http://lists.xensource.com/xen-devel > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Feb-16 10:02 UTC
[SPAM] Re: [Xen-devel] [PATCH] xenbus PM events support (was: [PATCH 2/2] Fix hangup after creating checkpoint on Xen.)
On Tue, 2011-02-15 at 23:10 +0000, Shriram Rajagopalan wrote:> diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c > b/drivers/xen/xenbus/xenbus_probe_frontend.c > index 5bcc2d6..9ad8868 100644 > --- a/drivers/xen/xenbus/xenbus_probe_frontend.c > +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c > @@ -85,6 +85,12 @@ static struct device_attribute > xenbus_frontend_dev_attrs[] = { > __ATTR_NULL > }; > > +static struct dev_pm_ops xenbus_pm_ops = { > + .suspend = xenbus_dev_suspend, > + .resume = xenbus_dev_resume, > + .thaw = xenbus_dev_cancel, > +}; > +Please run ./scripts/checkpatch.pl on patches, in this case it says: $ ./scripts/checkpatch.pl ~/1.mbox WARNING: struct dev_pm_ops should normally be const #167: FILE: drivers/xen/xenbus/xenbus_probe_frontend.c:88: +static struct dev_pm_ops xenbus_pm_ops = { total: 0 errors, 1 warnings, 69 lines checked /home/ianc/1.mbox has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. checkpatch.pl isn''t gospel but this seems like a reasonable warning to address. The following didn''t seem to provoke any compile time errors; diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c index d6e5f0d..b6a2690 100644 --- a/drivers/xen/xenbus/xenbus_probe_frontend.c +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c @@ -85,7 +85,7 @@ static struct device_attribute xenbus_frontend_dev_attrs[] = { __ATTR_NULL }; -static struct dev_pm_ops xenbus_pm_ops = { +static const struct dev_pm_ops xenbus_pm_ops = { .suspend = xenbus_dev_suspend, .resume = xenbus_dev_resume, .freeze = xenbus_dev_suspend, Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Shriram Rajagopalan
2011-Feb-16 19:07 UTC
Re: [Xen-devel] [PATCH] xenbus PM events support (was: [PATCH 2/2] Fix hangup after creating checkpoint on Xen.)
On Wed, Feb 16, 2011 at 1:43 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> > On Tue, 2011-02-15 at 23:20 +0000, Shriram Rajagopalan wrote: > > The above patch is against upstream/for-linus tree. > > In general it is better to pick either a mainline kernel (e.g. the > master branch from > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git), > one of the stable-* or next-* branches from xen.git or a specific topic > branch. The for-* branches are usually just throwaway branches which are > not necessarily kept up to date or relevant standalone. (in this case I > think it''s ok since it is just 2.6.38-rc1 + a little bit) > > > The xen-netfront driver does not have any suspend handler at all, in this tree. > > I checked other trees (upstream/pvhvm, upstream/xen, > > upstream/netfront). Same issue. > > This patch doesn''t appear to touch xen-netfront though? > > It looks like the netfront driver only needs to do work on resume, not > suspend. This is consistent with the explanation of why freeze/suspend > and thaw/resume are the same under Xen which you gave in an earlier > mail, isn''t it? > > A quick "git grep xenbus_driver" doesn''t show any of the other frontends > having a suspend callback either. >Which tree are you talking about? xen/next-2.6.32 and xen/next has netfront_suspend and I think Kazuhiro''s patch was against this. Looking at the netfront_suspend code from next-2.6.32, it simply cancels the smart poll timers on suspend. And this was introduced by commit 5473680bdedb7a62e641970119e6e9381a8d80f4 Author: Author: Dongxiao Xu <dongxiao.xu@intel.com> in order to fix save/restore when using smart poll timers. But xen/next-2.6.38 does not have this function. So I dont know why it has been removed.> > So, I dont know which tree Kazuhiro''s original patch applies to. > > > > Shriram > > > > On Tue, Feb 15, 2011 at 3:10 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote: > > > From: SUZUKI, Kazuhiro <kaz@jp.fujitsu.com> > > > > > > Hi, > > > > > > This is a Xen part patch. > > Can you improve on this description please. > > I meant to respond to this in the initial posting and forgot once the > thread got sidetracked into what the correct approach is. > > My advice to "just pipe the mail into git am" may have been a bit > misleading, sorry. > > In general it''s fine to fixup the description and/or minor nits with the > patch as it passes through your hand. If the changes are significant > it''s useful to acknowledge what you''ve done, many people add a > [shriram--improved commit message] > style tag below their signed off. > > In this case the actual patch itself is fine and: > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > Ian. > > > > > > > Thanks, > > > KAZ > > > > > > Signed-off-by: Kenji Wakamiya <wkenji@jp.fujitsu.com> > > > Signed-off-by: Kazuhiro Suzuki <kaz@jp.fujitsu.com> > > > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> > > > --- > > > drivers/xen/xenbus/xenbus_probe.c | 12 ++++++++++-- > > > drivers/xen/xenbus/xenbus_probe.h | 3 ++- > > > drivers/xen/xenbus/xenbus_probe_frontend.c | 9 +++++++-- > > > include/xen/xenbus.h | 2 +- > > > 4 files changed, 20 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c > > > index baa65e7..7397695 100644 > > > --- a/drivers/xen/xenbus/xenbus_probe.c > > > +++ b/drivers/xen/xenbus/xenbus_probe.c > > > @@ -577,7 +577,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; > > > @@ -590,7 +590,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); > > > @@ -642,6 +642,14 @@ int xenbus_dev_resume(struct device *dev) > > > } > > > EXPORT_SYMBOL_GPL(xenbus_dev_resume); > > > > > > +int xenbus_dev_cancel(struct device *dev) > > > +{ > > > + /* Do nothing */ > > > + DPRINTK("cancel"); > > > + return 0; > > > +} > > > +EXPORT_SYMBOL_GPL(xenbus_dev_cancel); > > > + > > > /* A flag to determine if xenstored is ''ready'' (i.e. has started) */ > > > int xenstored_ready = 0; > > > > > > diff --git a/drivers/xen/xenbus/xenbus_probe.h b/drivers/xen/xenbus/xenbus_probe.h > > > index 2466581..888b990 100644 > > > --- a/drivers/xen/xenbus/xenbus_probe.h > > > +++ b/drivers/xen/xenbus/xenbus_probe.h > > > @@ -64,8 +64,9 @@ 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_resume(struct device *dev); > > > +extern int xenbus_dev_cancel(struct device *dev); > > > > > > extern void xenbus_otherend_changed(struct xenbus_watch *watch, > > > const char **vec, unsigned int len, > > > diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c > > > index 5bcc2d6..9ad8868 100644 > > > --- a/drivers/xen/xenbus/xenbus_probe_frontend.c > > > +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c > > > @@ -85,6 +85,12 @@ static struct device_attribute xenbus_frontend_dev_attrs[] = { > > > __ATTR_NULL > > > }; > > > > > > +static struct dev_pm_ops xenbus_pm_ops = { > > > + .suspend = xenbus_dev_suspend, > > > + .resume = xenbus_dev_resume, > > > + .thaw = xenbus_dev_cancel, > > > +}; > > > + > > > static struct xen_bus_type xenbus_frontend = { > > > .root = "device", > > > .levels = 2, /* device/type/<id> */ > > > @@ -100,8 +106,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 7a1d15f..5467369 100644 > > > --- a/include/xen/xenbus.h > > > +++ b/include/xen/xenbus.h > > > @@ -92,7 +92,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; > > > -- > > > 1.7.0.4 > > > > > > > > > _______________________________________________ > > > Xen-devel mailing list > > > Xen-devel@lists.xensource.com > > > http://lists.xensource.com/xen-devel > > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xensource.com > > http://lists.xensource.com/xen-devel > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Feb-16 19:22 UTC
Re: [Xen-devel] [PATCH] xenbus PM events support (was: [PATCH 2/2] Fix hangup after creating checkpoint on Xen.)
On Wed, 2011-02-16 at 19:07 +0000, Shriram Rajagopalan wrote:> On Wed, Feb 16, 2011 at 1:43 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > > > On Tue, 2011-02-15 at 23:20 +0000, Shriram Rajagopalan wrote: > > > The above patch is against upstream/for-linus tree. > > > > In general it is better to pick either a mainline kernel (e.g. the > > master branch from > > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git), > > one of the stable-* or next-* branches from xen.git or a specific topic > > branch. The for-* branches are usually just throwaway branches which are > > not necessarily kept up to date or relevant standalone. (in this case I > > think it''s ok since it is just 2.6.38-rc1 + a little bit) > > > > > The xen-netfront driver does not have any suspend handler at all, in this tree. > > > I checked other trees (upstream/pvhvm, upstream/xen, > > > upstream/netfront). Same issue. > > > > This patch doesn''t appear to touch xen-netfront though? > > > > It looks like the netfront driver only needs to do work on resume, not > > suspend. This is consistent with the explanation of why freeze/suspend > > and thaw/resume are the same under Xen which you gave in an earlier > > mail, isn''t it? > > > > A quick "git grep xenbus_driver" doesn''t show any of the other frontends > > having a suspend callback either. > > > Which tree are you talking about? xen/next-2.6.32 and xen/next has > netfront_suspend and I think Kazuhiro''s patch was against this. > > Looking at the netfront_suspend code from next-2.6.32, it simply cancels the > smart poll timers on suspend. And this was introduced by > > commit 5473680bdedb7a62e641970119e6e9381a8d80f4 > Author: Author: Dongxiao Xu <dongxiao.xu@intel.com> > > in order to fix save/restore when using smart poll timers. > > But xen/next-2.6.38 does not have this function. So I dont know why it has been > removed.I was looking at an upstream tree. I thought I looked at 2.6.32 too but must have messed up. smartpoll was never upstreamed. AFAIK there are no current plans to do so. I think you can safely ignore it for the purposes of making these changes upstream. Ian.> > > > So, I dont know which tree Kazuhiro''s original patch applies to. > > > > > > Shriram > > > > > > On Tue, Feb 15, 2011 at 3:10 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote: > > > > From: SUZUKI, Kazuhiro <kaz@jp.fujitsu.com> > > > > > > > > Hi, > > > > > > > > This is a Xen part patch. > > > > Can you improve on this description please. > > > > I meant to respond to this in the initial posting and forgot once the > > thread got sidetracked into what the correct approach is. > > > > My advice to "just pipe the mail into git am" may have been a bit > > misleading, sorry. > > > > In general it''s fine to fixup the description and/or minor nits with the > > patch as it passes through your hand. If the changes are significant > > it''s useful to acknowledge what you''ve done, many people add a > > [shriram--improved commit message] > > style tag below their signed off. > > > > In this case the actual patch itself is fine and: > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > > > Ian. > > > > > > > > > > Thanks, > > > > KAZ > > > > > > > > Signed-off-by: Kenji Wakamiya <wkenji@jp.fujitsu.com> > > > > Signed-off-by: Kazuhiro Suzuki <kaz@jp.fujitsu.com> > > > > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> > > > > --- > > > > drivers/xen/xenbus/xenbus_probe.c | 12 ++++++++++-- > > > > drivers/xen/xenbus/xenbus_probe.h | 3 ++- > > > > drivers/xen/xenbus/xenbus_probe_frontend.c | 9 +++++++-- > > > > include/xen/xenbus.h | 2 +- > > > > 4 files changed, 20 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c > > > > index baa65e7..7397695 100644 > > > > --- a/drivers/xen/xenbus/xenbus_probe.c > > > > +++ b/drivers/xen/xenbus/xenbus_probe.c > > > > @@ -577,7 +577,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; > > > > @@ -590,7 +590,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); > > > > @@ -642,6 +642,14 @@ int xenbus_dev_resume(struct device *dev) > > > > } > > > > EXPORT_SYMBOL_GPL(xenbus_dev_resume); > > > > > > > > +int xenbus_dev_cancel(struct device *dev) > > > > +{ > > > > + /* Do nothing */ > > > > + DPRINTK("cancel"); > > > > + return 0; > > > > +} > > > > +EXPORT_SYMBOL_GPL(xenbus_dev_cancel); > > > > + > > > > /* A flag to determine if xenstored is ''ready'' (i.e. has started) */ > > > > int xenstored_ready = 0; > > > > > > > > diff --git a/drivers/xen/xenbus/xenbus_probe.h b/drivers/xen/xenbus/xenbus_probe.h > > > > index 2466581..888b990 100644 > > > > --- a/drivers/xen/xenbus/xenbus_probe.h > > > > +++ b/drivers/xen/xenbus/xenbus_probe.h > > > > @@ -64,8 +64,9 @@ 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_resume(struct device *dev); > > > > +extern int xenbus_dev_cancel(struct device *dev); > > > > > > > > extern void xenbus_otherend_changed(struct xenbus_watch *watch, > > > > const char **vec, unsigned int len, > > > > diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c > > > > index 5bcc2d6..9ad8868 100644 > > > > --- a/drivers/xen/xenbus/xenbus_probe_frontend.c > > > > +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c > > > > @@ -85,6 +85,12 @@ static struct device_attribute xenbus_frontend_dev_attrs[] = { > > > > __ATTR_NULL > > > > }; > > > > > > > > +static struct dev_pm_ops xenbus_pm_ops = { > > > > + .suspend = xenbus_dev_suspend, > > > > + .resume = xenbus_dev_resume, > > > > + .thaw = xenbus_dev_cancel, > > > > +}; > > > > + > > > > static struct xen_bus_type xenbus_frontend = { > > > > .root = "device", > > > > .levels = 2, /* device/type/<id> */ > > > > @@ -100,8 +106,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 7a1d15f..5467369 100644 > > > > --- a/include/xen/xenbus.h > > > > +++ b/include/xen/xenbus.h > > > > @@ -92,7 +92,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; > > > > -- > > > > 1.7.0.4 > > > > > > > > > > > > _______________________________________________ > > > > Xen-devel mailing list > > > > Xen-devel@lists.xensource.com > > > > http://lists.xensource.com/xen-devel > > > > > > > > > > _______________________________________________ > > > Xen-devel mailing list > > > Xen-devel@lists.xensource.com > > > http://lists.xensource.com/xen-devel > > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Shriram Rajagopalan
2011-Feb-16 20:15 UTC
[Xen-devel] [PATCH] xen: xenbus PM events support
From: SUZUKI, Kazuhiro <kaz@jp.fujitsu.com>
Make xenbus frontend device subscribe to PM events
to receive suspend/resume/freeze/thaw/restore
notifications.
Signed-off-by: Kenji Wakamiya <wkenji@jp.fujitsu.com>
Signed-off-by: Kazuhiro Suzuki <kaz@jp.fujitsu.com>
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
[shriram--minor mods and improved commit message]
---
drivers/xen/xenbus/xenbus_probe.c | 12 ++++++++++--
drivers/xen/xenbus/xenbus_probe.h | 3 ++-
drivers/xen/xenbus/xenbus_probe_frontend.c | 9 +++++++--
include/xen/xenbus.h | 2 +-
4 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus_probe.c
b/drivers/xen/xenbus/xenbus_probe.c
index baa65e7..7397695 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -577,7 +577,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;
@@ -590,7 +590,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);
@@ -642,6 +642,14 @@ int xenbus_dev_resume(struct device *dev)
}
EXPORT_SYMBOL_GPL(xenbus_dev_resume);
+int xenbus_dev_cancel(struct device *dev)
+{
+ /* Do nothing */
+ DPRINTK("cancel");
+ return 0;
+}
+EXPORT_SYMBOL_GPL(xenbus_dev_cancel);
+
/* A flag to determine if xenstored is ''ready'' (i.e. has
started) */
int xenstored_ready = 0;
diff --git a/drivers/xen/xenbus/xenbus_probe.h
b/drivers/xen/xenbus/xenbus_probe.h
index 2466581..888b990 100644
--- a/drivers/xen/xenbus/xenbus_probe.h
+++ b/drivers/xen/xenbus/xenbus_probe.h
@@ -64,8 +64,9 @@ 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_resume(struct device *dev);
+extern int xenbus_dev_cancel(struct device *dev);
extern void xenbus_otherend_changed(struct xenbus_watch *watch,
const char **vec, unsigned int len,
diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c
b/drivers/xen/xenbus/xenbus_probe_frontend.c
index 5bcc2d6..ea83999 100644
--- a/drivers/xen/xenbus/xenbus_probe_frontend.c
+++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
@@ -85,6 +85,12 @@ static struct device_attribute xenbus_frontend_dev_attrs[] =
{
__ATTR_NULL
};
+static const struct dev_pm_ops xenbus_pm_ops = {
+ .suspend = xenbus_dev_suspend,
+ .resume = xenbus_dev_resume,
+ .thaw = xenbus_dev_cancel,
+};
+
static struct xen_bus_type xenbus_frontend = {
.root = "device",
.levels = 2, /* device/type/<id> */
@@ -100,8 +106,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 7a1d15f..5467369 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -92,7 +92,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;
--
1.7.0.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel