From: Anchal Agarwal <anchalag@amazon.com>
To: <boris.ostrovsky@oracle.com>
Cc: <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>,
<hpa@zytor.com>, <x86@kernel.org>, <jgross@suse.com>,
<linux-pm@vger.kernel.org>, <linux-mm@kvack.org>,
<kamatam@amazon.com>, <sstabellini@kernel.org>,
<konrad.wilk@oracle.com>, <roger.pau@citrix.com>,
<axboe@kernel.dk>, <davem@davemloft.net>, <rjw@rjwysocki.net>,
<len.brown@intel.com>, <pavel@ucw.cz>, <peterz@infradead.org>,
<eduval@amazon.com>, <sblbir@amazon.com>,
<xen-devel@lists.xenproject.org>, <vkuznets@redhat.com>,
<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<dwmw@amazon.co.uk>, <benh@kernel.crashing.org>
Subject: Re: [PATCH v3 02/11] xenbus: add freeze/thaw/restore callbacks support
Date: Tue, 15 Sep 2020 19:56:43 +0000 [thread overview]
Message-ID: <20200915195643.GA28542@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com> (raw)
In-Reply-To: <eebc26b8-f1b1-3bea-5366-dd77f063237e@oracle.com>
On Sun, Sep 13, 2020 at 12:11:47PM -0400, boris.ostrovsky@oracle.com wrote:
> 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 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.
>
The message is trying to say that that freeze/thaw/restore callbacks will be
used for both PM SUSPEND and PM HIBERNATION. Since, we are only focussing on PM
hibernation, I will remove all wordings of PM suspend from this message to avoid
confusion. I left it there in case someone wants to pick it up in future knowing
framework is already present.
>
> And please be consistent with "PM suspend" vs. "PM hibernation".
>
I should remove PM suspend from everywhere since the mode is not tested
for.
>
> > 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)?
>
>
Nope, they should be same. We can use dev here too. I should probably just use
dev.
> > "%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.
>
Yes this I left unchanged, it should be consistent with xdev.
>
> > + bool xen_suspend = is_xen_suspend();
>
>
> No need for this, you use it only once anyway.
>
>
> -boris
>
Thanks,
Anchal
>
next prev parent reply other threads:[~2020-09-15 22:11 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-21 22:22 [PATCH v3 00/11] Fix PM hibernation in Xen guests Anchal Agarwal
2020-08-21 22:25 ` [PATCH v3 01/11] xen/manage: keep track of the on-going suspend mode Anchal Agarwal
2020-09-13 15:43 ` boris.ostrovsky
2020-09-14 21:47 ` Anchal Agarwal
2020-09-15 0:24 ` boris.ostrovsky
2020-09-15 18:00 ` Anchal Agarwal
2020-09-15 19:58 ` boris.ostrovsky
2020-09-21 21:54 ` Anchal Agarwal
2020-09-22 16:18 ` boris.ostrovsky
2020-09-22 23:17 ` Anchal Agarwal
[not found] ` <20200925190423.GA31885@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com>
2020-09-25 20:02 ` boris.ostrovsky
2020-09-25 22:28 ` Anchal Agarwal
2020-09-28 18:49 ` boris.ostrovsky
2020-09-30 21:29 ` Anchal Agarwal
2020-10-01 12:43 ` boris.ostrovsky
2021-05-21 5:26 ` Anchal Agarwal
2021-05-25 22:23 ` Boris Ostrovsky
2021-05-26 4:40 ` Anchal Agarwal
2021-05-26 18:29 ` Boris Ostrovsky
[not found] ` <20210528215008.GA19622@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com>
2021-06-01 14:18 ` Boris Ostrovsky
2021-06-02 19:37 ` Anchal Agarwal
2021-06-03 20:11 ` Boris Ostrovsky
2021-06-03 23:27 ` Anchal Agarwal
2021-06-04 1:49 ` Boris Ostrovsky
2020-09-13 17:07 ` boris.ostrovsky
2020-08-21 22:26 ` [PATCH v3 02/11] xenbus: add freeze/thaw/restore callbacks support Anchal Agarwal
2020-09-13 16:11 ` boris.ostrovsky
2020-09-15 19:56 ` Anchal Agarwal [this message]
2020-08-21 22:26 ` [PATCH v3 03/11] x86/xen: Introduce new function to map HYPERVISOR_shared_info on Resume Anchal Agarwal
2020-08-21 22:27 ` [PATCH v3 04/11] x86/xen: add system core suspend and resume callbacks Anchal Agarwal
2020-09-13 17:25 ` boris.ostrovsky
2020-08-21 22:27 ` [PATCH v3 05/11] genirq: Shutdown irq chips in suspend/resume during hibernation Thomas Gleixner
2020-08-22 0:36 ` Thomas Gleixner
2020-08-24 17:25 ` Anchal Agarwal
2020-08-25 13:20 ` Christoph Hellwig
2020-08-25 15:25 ` Thomas Gleixner
2020-08-21 22:28 ` [PATCH v3 06/11] xen-blkfront: add callbacks for PM suspend and hibernation Anchal Agarwal
2020-08-21 22:29 ` [PATCH v3 07/11] xen-netfront: " Anchal Agarwal
2020-08-21 22:29 ` [PATCH v3 08/11] x86/xen: save and restore steal clock during PM hibernation Anchal Agarwal
2020-08-21 22:30 ` [PATCH v3 09/11] xen: Introduce wrapper for save/restore sched clock offset Anchal Agarwal
2020-08-21 22:30 ` [PATCH v3 10/11] xen: Update sched clock offset to avoid system instability in hibernation Anchal Agarwal
2020-09-13 17:52 ` boris.ostrovsky
2020-08-21 22:31 ` [PATCH v3 11/11] PM / hibernate: update the resume offset on SNAPSHOT_SET_SWAP_AREA Anchal Agarwal
2020-08-28 18:26 ` [PATCH v3 00/11] Fix PM hibernation in Xen guests Anchal Agarwal
2020-08-28 18:29 ` Rafael J. Wysocki
2020-08-28 18:39 ` Anchal Agarwal
2020-09-11 20:44 ` Anchal Agarwal
2020-09-11 15:19 ` boris.ostrovsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200915195643.GA28542@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com \
--to=anchalag@amazon.com \
--cc=axboe@kernel.dk \
--cc=benh@kernel.crashing.org \
--cc=boris.ostrovsky@oracle.com \
--cc=bp@alien8.de \
--cc=davem@davemloft.net \
--cc=dwmw@amazon.co.uk \
--cc=eduval@amazon.com \
--cc=hpa@zytor.com \
--cc=jgross@suse.com \
--cc=kamatam@amazon.com \
--cc=konrad.wilk@oracle.com \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-pm@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=peterz@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=roger.pau@citrix.com \
--cc=sblbir@amazon.com \
--cc=sstabellini@kernel.org \
--cc=tglx@linutronix.de \
--cc=vkuznets@redhat.com \
--cc=x86@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).