diff mbox series

[02/12] xenbus: add freeze/thaw/restore callbacks support

Message ID 7fd12227f923eacc5841b47bd69f72b4105843a7.1589926004.git.anchalag@amazon.com (mailing list archive)
State Superseded
Headers show
Series Fix PM hibernation in Xen guests | expand

Commit Message

Anchal Agarwal May 19, 2020, 11:25 p.m. UTC 
From: Munehisa Kamata <kamatam@amazon.com>

Since commit b3e96c0c7562 ("xen: use freeze/restore/thaw PM events for
suspend/resume/chkpt"), xenbus uses PMSG_FREEZE, PMSG_THAW and
PMSG_RESTORE events for Xen suspend. However, they're actually assigned
to xenbus_dev_suspend(), xenbus_dev_cancel() and xenbus_dev_resume()
respectively, and only suspend and resume callbacks are supported at
driver level. To support PM suspend and PM hibernation, modify the bus
level PM callbacks to invoke not only device driver's suspend/resume but
also freeze/thaw/restore.

Note that we'll use freeze/restore callbacks even for PM suspend whereas
suspend/resume callbacks are normally used in the case, becausae the
existing xenbus device drivers already have suspend/resume callbacks
specifically designed for Xen suspend. So we can allow the device
drivers to keep the existing callbacks wihtout modification.

[Anchal Changelog: Refactored the callbacks code]
Signed-off-by: Agarwal Anchal <anchalag@amazon.com>
Signed-off-by: Munehisa Kamata <kamatam@amazon.com>
---
 drivers/xen/xenbus/xenbus_probe.c | 99 +++++++++++++++++++++++++------
 include/xen/xenbus.h              |  3 +
 2 files changed, 84 insertions(+), 18 deletions(-)

Comments

Boris Ostrovsky May 30, 2020, 10:56 p.m. UTC | #1 
On 5/19/20 7:25 PM, Anchal Agarwal wrote:
>  
>  int xenbus_dev_resume(struct device *dev)
>  {
> -	int err;
> +	int err = 0;


That's not necessary.


>  	struct xenbus_driver *drv;
>  	struct xenbus_device *xdev
>  		= container_of(dev, struct xenbus_device, dev);
> -
> +	bool xen_suspend = xen_suspend_mode_is_xen_suspend();
>  	DPRINTK("%s", xdev->nodename);
>  
>  	if (dev->driver == NULL)
> @@ -627,24 +645,32 @@ int xenbus_dev_resume(struct device *dev)
>  	drv = to_xenbus_driver(dev->driver);
>  	err = talk_to_otherend(xdev);
>  	if (err) {
> -		pr_warn("resume (talk_to_otherend) %s failed: %i\n",
> +		pr_warn("%s (talk_to_otherend) %s failed: %i\n",


Please use dev_warn() everywhere, we just had a bunch of patches that
replaced pr_warn(). In fact,  this is one of the lines that got changed.


>  
>  int xenbus_dev_cancel(struct device *dev)
>  {
> -	/* Do nothing */
> -	DPRINTK("cancel");
> +	int err = 0;


Again, no need to initialize.


> +	struct xenbus_driver *drv;
> +	struct xenbus_device *xdev
> +		= container_of(dev, struct xenbus_device, dev);


xendev please to be consistent with other code. And use to_xenbus_device().


-boris
Anchal Agarwal June 1, 2020, 11:36 p.m. UTC | #2 
    CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.



    On 5/19/20 7:25 PM, Anchal Agarwal wrote:
    >
    >  int xenbus_dev_resume(struct device *dev)
    >  {
    > -     int err;
    > +     int err = 0;


    That's not necessary.
ACK.

    >       struct xenbus_driver *drv;
    >       struct xenbus_device *xdev
    >               = container_of(dev, struct xenbus_device, dev);
    > -
    > +     bool xen_suspend = xen_suspend_mode_is_xen_suspend();
    >       DPRINTK("%s", xdev->nodename);
    >
    >       if (dev->driver == NULL)
    > @@ -627,24 +645,32 @@ int xenbus_dev_resume(struct device *dev)
    >       drv = to_xenbus_driver(dev->driver);
    >       err = talk_to_otherend(xdev);
    >       if (err) {
    > -             pr_warn("resume (talk_to_otherend) %s failed: %i\n",
    > +             pr_warn("%s (talk_to_otherend) %s failed: %i\n",


    Please use dev_warn() everywhere, we just had a bunch of patches that
    replaced pr_warn(). In fact,  this is one of the lines that got changed.

ACK. Will send fixes in next series

    >
    >  int xenbus_dev_cancel(struct device *dev)
    >  {
    > -     /* Do nothing */
    > -     DPRINTK("cancel");
    > +     int err = 0;


    Again, no need to initialize.

ACK.
    > +     struct xenbus_driver *drv;
    > +     struct xenbus_device *xdev
    > +             = container_of(dev, struct xenbus_device, dev);


    xendev please to be consistent with other code. And use to_xenbus_device().
ACK.

    -boris

I will put the fixes in next round of patches.

Thanks,
Anchal
diff mbox series

Patch

diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 8c4d05b687b7..1589b9b2cb56 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -49,6 +49,7 @@ 
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/suspend.h>
 
 #include <asm/page.h>
 #include <asm/pgtable.h>
@@ -599,27 +600,44 @@  int xenbus_dev_suspend(struct device *dev)
 	struct xenbus_driver *drv;
 	struct xenbus_device *xdev
 		= container_of(dev, struct xenbus_device, dev);
-
+	bool xen_suspend = xen_suspend_mode_is_xen_suspend();
 	DPRINTK("%s", xdev->nodename);
 
 	if (dev->driver == NULL)
 		return 0;
 	drv = to_xenbus_driver(dev->driver);
-	if (drv->suspend)
-		err = drv->suspend(xdev);
-	if (err)
-		pr_warn("suspend %s failed: %i\n", dev_name(dev), err);
+
+	if (xen_suspend) {
+		if (drv->suspend)
+			err = drv->suspend(xdev);
+	} else {
+		if (drv->freeze) {
+			err = drv->freeze(xdev);
+			if (!err) {
+				free_otherend_watch(xdev);
+				free_otherend_details(xdev);
+				return 0;
+			}
+		}
+	}
+
+	if (err) {
+		pr_warn("%s %s failed: %i\n", xen_suspend ?
+			"suspend" : "freeze", dev_name(dev), err);
+		return err;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(xenbus_dev_suspend);
 
 int xenbus_dev_resume(struct device *dev)
 {
-	int err;
+	int err = 0;
 	struct xenbus_driver *drv;
 	struct xenbus_device *xdev
 		= container_of(dev, struct xenbus_device, dev);
-
+	bool xen_suspend = xen_suspend_mode_is_xen_suspend();
 	DPRINTK("%s", xdev->nodename);
 
 	if (dev->driver == NULL)
@@ -627,24 +645,32 @@  int xenbus_dev_resume(struct device *dev)
 	drv = to_xenbus_driver(dev->driver);
 	err = talk_to_otherend(xdev);
 	if (err) {
-		pr_warn("resume (talk_to_otherend) %s failed: %i\n",
+		pr_warn("%s (talk_to_otherend) %s failed: %i\n",
+			xen_suspend ? "resume" : "restore",
 			dev_name(dev), err);
 		return err;
 	}
 
-	xdev->state = XenbusStateInitialising;
+	if (xen_suspend) {
+		xdev->state = XenbusStateInitialising;
+		if (drv->resume)
+			err = drv->resume(xdev);
+	} else {
+		if (drv->restore)
+			err = drv->restore(xdev);
+	}
 
-	if (drv->resume) {
-		err = drv->resume(xdev);
-		if (err) {
-			pr_warn("resume %s failed: %i\n", dev_name(dev), err);
-			return err;
-		}
+	if (err) {
+		pr_warn("%s %s failed: %i\n",
+			xen_suspend ? "resume" : "restore",
+			dev_name(dev), err);
+		return err;
 	}
 
 	err = watch_otherend(xdev);
 	if (err) {
-		pr_warn("resume (watch_otherend) %s failed: %d.\n",
+		pr_warn("%s (watch_otherend) %s failed: %d.\n",
+			xen_suspend ? "resume" : "restore",
 			dev_name(dev), err);
 		return err;
 	}
@@ -655,8 +681,45 @@  EXPORT_SYMBOL_GPL(xenbus_dev_resume);
 
 int xenbus_dev_cancel(struct device *dev)
 {
-	/* Do nothing */
-	DPRINTK("cancel");
+	int err = 0;
+	struct xenbus_driver *drv;
+	struct xenbus_device *xdev
+		= container_of(dev, struct xenbus_device, dev);
+	bool xen_suspend = xen_suspend_mode_is_xen_suspend();
+
+	if (xen_suspend) {
+		/* Do nothing */
+		DPRINTK("cancel");
+		return 0;
+	}
+
+	DPRINTK("%s", xdev->nodename);
+
+	if (dev->driver == NULL)
+		return 0;
+	drv = to_xenbus_driver(dev->driver);
+	err = talk_to_otherend(xdev);
+	if (err) {
+		pr_warn("thaw (talk_to_otherend) %s failed: %d.\n",
+			dev_name(dev), err);
+		return err;
+	}
+
+	if (drv->thaw) {
+		err = drv->thaw(xdev);
+		if (err) {
+			pr_warn("thaw %s failed: %i\n", dev_name(dev), err);
+			return err;
+		}
+	}
+
+	err = watch_otherend(xdev);
+	if (err) {
+		pr_warn("thaw (watch_otherend) %s failed: %d.\n",
+			dev_name(dev), err);
+		return err;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(xenbus_dev_cancel);
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index 5a8315e6d8a6..8da964763255 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -104,6 +104,9 @@  struct xenbus_driver {
 	int (*remove)(struct xenbus_device *dev);
 	int (*suspend)(struct xenbus_device *dev);
 	int (*resume)(struct xenbus_device *dev);
+	int (*freeze)(struct xenbus_device *dev);
+	int (*thaw)(struct xenbus_device *dev);
+	int (*restore)(struct xenbus_device *dev);
 	int (*uevent)(struct xenbus_device *, struct kobj_uevent_env *);
 	struct device_driver driver;
 	int (*read_otherend_details)(struct xenbus_device *dev);