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

[Aurelien Chartier <aurelien.chartier@xxxxxxxxxx>]

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 ?

>
>> --- 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..."

Right. I will fix it.
>
>> +    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.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel