Re: [PATCH V3 2/2] xenbus: delay xenbus frontend resume is xenstored is not running

[David Vrabel <david.vrabel@citrix.com>]

On 14/05/13 14:04, Aurelien Chartier wrote:
> On 14/05/13 11:09, David Vrabel wrote:
>> 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?
> Linux driver resume code is calling the resume function defined in the
> dev_pm_ops field of the bus. I don't see any way to have that resume
> done at the bus level for xenbus, but I may have missed something ?

No, I was misremembering.

>>> +	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.
> 
> Yes, that will be blocked until xenstored process has been resumed,
> since it is the current behavior. However, as the process resume is now
> happening in parallel, it will only stay blocked for a very short period
> of time.

This may cause deadlocks.

It used to be that a work item could block the execution of other work
on that workqueue by sleeping or not completing,  but workqueues have
changed quite a bit since I last looked at them detail so this might not
be the case any more.

As resuming N devices queues N work items this can cause N worker
threads to block.  Is it guaranteed that the kernel can continue to
create more workers to handle any work items required before xenstored
can start?

The safe and obviously correct method would be to create a dedicated
workqueue for this resume work.

David