From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aurelien Chartier Subject: Re: [PATCH V2 2/2] xenbus: defer xenbus frontend resume if xenstored is not running Date: Wed, 8 May 2013 15:40:19 +0100 Message-ID: <518A63D3.5080002@citrix.com> References: <1368023557-21538-1-git-send-email-aurelien.chartier@citrix.com> <1368023557-21538-3-git-send-email-aurelien.chartier@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1368023557-21538-3-git-send-email-aurelien.chartier@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Aurelien Chartier Cc: Stefano Stabellini , "xen-devel@lists.xensource.com" , Ian Campbell , "JBeulich@suse.com" , "konrad.wilk@oracle.com" List-Id: xen-devel@lists.xenproject.org On 08/05/13 15:32, Aurelien Chartier wrote: > If the xenbus frontend is running in a domain running xenstored, the device > resume is hanging because it is happening before the process resume. This > patch adds extra logic to the resume code to check if we are the domain > running xenstored and delay the resume if needed. > > Signed-off-by: Aurelien Chartier > > Changes in v2: > - Instead of bypassing the resume, process it in a workqueue > --- > drivers/xen/xenbus/xenbus_probe.h | 5 ++++ > drivers/xen/xenbus/xenbus_probe_frontend.c | 34 +++++++++++++++++++++++++++- > 2 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/drivers/xen/xenbus/xenbus_probe.h b/drivers/xen/xenbus/xenbus_probe.h > index 146f857..46b384c 100644 > --- a/drivers/xen/xenbus/xenbus_probe.h > +++ b/drivers/xen/xenbus/xenbus_probe.h > @@ -47,6 +47,11 @@ struct xen_bus_type { > struct bus_type bus; > }; > > +struct xenbus_resume_work { > + struct work_struct w; > + struct device *dev; > +}; > + > enum xenstore_init { > XS_UNKNOWN, > XS_PV, > diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c > index 3159a37..83b83d0 100644 > --- a/drivers/xen/xenbus/xenbus_probe_frontend.c > +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c > @@ -28,6 +28,7 @@ > #include "xenbus_comms.h" > #include "xenbus_probe.h" > > +static struct workqueue_struct *xenbus_frontend_resume_wq; > > /* device// => - */ > static int frontend_bus_id(char bus_id[XEN_BUS_ID_SIZE], const char *nodename) > @@ -89,9 +90,38 @@ static void backend_changed(struct xenbus_watch *watch, > xenbus_otherend_changed(watch, vec, len, 1); > } > > +static void xenbus_frontend_delayed_resume(struct work_struct *w) > +{ > + struct xenbus_resume_work *resume_work = (struct xenbus_resume_work *) w; > + > + xenbus_dev_resume(dev); > + > + kfree(w); > +} > + > +static int xenbus_frontend_dev_resume(struct device *dev) > +{ > + /* > + * If xenstored is running in that domain, we cannot access the backend > + * state at the moment, so we need to defer xenbus_dev_resume > + */ > + if (xen_store_domain == XS_LOCAL) { > + struct xenbus_resume_work *work = > + kmalloc(sizeof(struct xenbus_resume), GFP_KERNEL); > + > + INIT_WORK((struct work_struct *) work, xenbus_frontend_delayed_resume); > + resume_work->dev = dev; > + queue_work(xenbus_frontend_resume_wq, (struct work_struct *) work) Missing a semicolon here. I should have read that patch once more, apologies for that. > + > + return 0; > + } > + > + return xenbus_dev_resume(dev); > +} > + > static const struct dev_pm_ops xenbus_pm_ops = { > .suspend = xenbus_dev_suspend, > - .resume = xenbus_dev_resume, > + .resume = xenbus_frontend_dev_resume, > .freeze = xenbus_dev_suspend, > .thaw = xenbus_dev_cancel, > .restore = xenbus_dev_resume, > @@ -440,6 +470,8 @@ static int __init xenbus_probe_frontend_init(void) > > register_xenstore_notifier(&xenstore_notifier); > > + xenbus_frontend_resume_wq = create_workqueue("xenbus_frontend_resume"); > + > return 0; > } > subsys_initcall(xenbus_probe_frontend_init);