Re: [PATCH V2 2/2] xenbus: defer xenbus frontend resume if xenstored is not running

[Aurelien Chartier <aurelien.chartier@citrix.com>]

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 <aurelien.chartier@citrix.com>
>
> 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/<type>/<id> => <type>-<id> */
>  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);