[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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@xxxxxxxxxx> 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@xxxxxxxxx>
> 
> 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@xxxxxxxxx> 
> > > wrote:
> > > > From: SUZUKI, Kazuhiro <kaz@xxxxxxxxxxxxxx>
> > > >
> > > > 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@xxxxxxxxxx>
> >
> > Ian.
> >
> > > >
> > > > Thanks,
> > > > KAZ
> > > >
> > > > Signed-off-by: Kenji Wakamiya <wkenji@xxxxxxxxxxxxxx>
> > > > Signed-off-by: Kazuhiro Suzuki <kaz@xxxxxxxxxxxxxx>
> > > > Signed-off-by: Shriram Rajagopalan <rshriram@xxxxxxxxx>
> > > > ---
> > > >  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@xxxxxxxxxxxxxxxxxxx
> > > > http://lists.xensource.com/xen-devel
> > > >
> > >
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@xxxxxxxxxxxxxxxxxxx
> > > http://lists.xensource.com/xen-devel
> >
> >



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.