lkml.org 
[lkml]   [2020]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 02/11] xenbus: add freeze/thaw/restore callbacks support
From
Date

On 8/21/20 6:26 PM, Anchal Agarwal wrote:
> 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.


Something is wrong with this sentence. Or with my brain --- I can't
quite parse this.


And please be consistent with "PM suspend" vs. "PM hibernation".


> So we can allow the device
> drivers to keep the existing callbacks wihtout modification.
>


> @@ -599,16 +600,33 @@ 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 = 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)
> - dev_warn(dev, "suspend failed: %i\n", err);
> + if (xen_suspend) {
> + if (drv->suspend)
> + err = drv->suspend(xdev);
> + } else {
> + if (drv->freeze) {


'else if' (to avoid extra indent level).  In xenbus_dev_resume() too.


> + err = drv->freeze(xdev);
> + if (!err) {
> + free_otherend_watch(xdev);
> + free_otherend_details(xdev);
> + return 0;
> + }
> + }
> + }
> +
> + if (err) {
> + dev_warn(&xdev->dev,


Is there a reason why you replaced dev with xdev->dev (here and elsewhere)?


> "%s %s failed: %d\n", xen_suspend ?
> + "suspend" : "freeze", xdev->nodename, err);
> + return err;
> + }
> +
>

> @@ -653,8 +683,44 @@ EXPORT_SYMBOL_GPL(xenbus_dev_resume);
>
> int xenbus_dev_cancel(struct device *dev)
> {
> - /* Do nothing */
> - DPRINTK("cancel");
> + int err;
> + struct xenbus_driver *drv;
> + struct xenbus_device *xendev = to_xenbus_device(dev);


xdev for consistency please.


> + bool xen_suspend = is_xen_suspend();


No need for this, you use it only once anyway.


-boris


\
 
 \ /
  Last update: 2020-09-13 18:13    [W:0.249 / U:9.376 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site