From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Vrabel <david.vrabel@citrix.com>
Subject: Re: [PATCH V3 2/2] xenbus: delay xenbus frontend resume
 is	xenstored is not running
Date: Tue, 14 May 2013 11:09:18 +0100
Message-ID: <51920D4E.9060906@citrix.com>
References: <1368450793-10030-1-git-send-email-aurelien.chartier@citrix.com>
	<1368450793-10030-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: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1368450793-10030-3-git-send-email-aurelien.chartier@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Aurelien Chartier <aurelien.chartier@citrix.com>
Cc: konrad.wilk@oracle.com, xen-devel@lists.xensource.com, ian.campbell@citrix.com, JBeulich@suse.com, stefano.stabellini@eu.citrix.com
List-Id: xen-devel@lists.xenproject.org

On 13/05/13 14:13, Aurelien Chartier wrote:
> If the xenbus frontend is located 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.

Would this be better done at the bus level instead of per-device?

> --- a/drivers/xen/xenbus/xenbus_probe_frontend.c
> +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
> @@ -89,9 +89,33 @@ 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_device *xdev = container_of(w, struct xenbus_device, work);
> +
> +	xenbus_dev_resume(&xdev->dev);
> +}
> +
> +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
> +	 */

I think you mean "If xenstored is running in /this/ domain, we cannot
access the backend state at the moment..."

> +	if (xen_store_domain == XS_LOCAL) {
> +		struct xenbus_device *xdev = to_xenbus_device(dev);
> +
> +		INIT_WORK(&xdev->work, xenbus_frontend_delayed_resume);
> +		schedule_work(&xdev->work);

How does this ensure xenstored is running when the work is scheduled?
Will it end up blocking on a system workqueue until xenstored is
running?  That would be bad.

David