Re: [PATCH] x86/xen: resume timer irqs early

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

On 08/08/14 18:15, Konrad Rzeszutek Wilk wrote:
> On Fri, Aug 08, 2014 at 03:35:27PM +0100, David Vrabel wrote:
>> On 08/08/14 15:04, Boris Ostrovsky wrote:
>>> On 08/08/2014 06:41 AM, David Vrabel wrote:
>>>> On 07/08/14 18:29, Boris Ostrovsky wrote:
>>>>> On 08/07/2014 01:16 PM, David Vrabel wrote:
>>>>>> If the timer irqs are resumed during device resume it is possible in
>>>>>> certain circumstances for the resume to hang early on, before device
>>>>>> interrupts are resumed.
>>>>>>
>>>>>> It is not entirely clear what is occuring the point of the hang but I
>>>>>> think a task necessary for the resume calls schedule_timeout(),
>>>>>> waiting for a timer interrupt (which never arrives).  This failure may
>>>>>> require specific tasks to be running on the other VCPUs to trigger
>>>>>> (processes are not frozen during a suspend/resume if PREEMPT is
>>>>>> disabled).
>>>>>>
>>>>>> Add IRQF_EARLY_RESUME to the timer interrupts so they are resumed in
>>>>>> syscore_resume().
>>>>>>
>>>>>> Also add IRQF_NO_SUSPEND as it is not necessary to suspend the timer
>>>>>> interrupts and IRQF_FORCE_RESUME was already set.
>>>>>
>>>>> IRQF_NO_SUSPEND is a component of IRQF_TIMER.
>>>> So it is.  How about this instead?
>>>
>>> The change makes sense so
>>>
>>>     Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>>
>>> but I am curious whether you actually were able to prove that it in fact
>>> fixes the hang (the description doesn't make it clear).
>>
>> Without the patch repeatedly migrating a VM would hang during resume
>> after < 500 iterations.  With the patch the VM was migrated > 8000 times
>> without a problem.
> 
> Ah, should said patch have a Reported-by too then?

I don't think the XenServer automated test system really minds.

> It would also be neat to have that in the description of the patch I think.

I'm not really keen on system-specific numbers like this, but it would
probably be useful in this case since my analysis is so woolly.

David