Re: [Xen-devel] [PATCH 1/2] common/kexec: Prevent deadlock on reentry to the crash path.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.11.13 at 14:30, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 25/11/13 13:28, Jan Beulich wrote:
>>>>> On 15.11.13 at 21:32, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>>> In some cases, such as suffering a queued-invalidation timeout while
>>> performing an iommu_crash_shutdown(), Xen can end up reentering the crash
>>> path. Previously, this would result in a deadlock in one_cpu_only(), as the
>>> test_and_set_bit() would fail.
>>>
>>> The crash path is not reentrant, and even if it could be made to be so, it 
> is
>>> almost certain that we would fall over the same reentry condition again.
>>>
>>> The new code can distinguish a reentry case from multiple cpus racing down 
> the
>>> crash path.  In the case that a reentry is detected, return back out to the
>>> nested panic() call, which will maybe_reboot() on our behalf.  This 
>>> requires 
> a
>>> bit of return plumbing back up to kexec_crash().
>>>
>>> While fixing this deadlock, also fix up an minor niggle seen recently from a
>>> XenServer crash report.  The report was from a Bank 8 MCE, which had managed
>>> to crash on all cpus at once.  The result was a lot of stack traces with 
> cpus
>>> in kexec_common_shutdown(), which was infact the inlined version of
>>> one_cpu_only().  The kexec crash path is not a hotpath, so we can easily
>>> afford to prevent inlining for the sake of clarity in the stack traces.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> CC: Keir Fraser <keir@xxxxxxx>
>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>> CC: Tim Deegan <tim@xxxxxxx>
>>> CC: David Vrabel <david.vrabel@xxxxxxxxxx>
>>> ---
>>>  xen/common/kexec.c |   51 
>>> ++++++++++++++++++++++++++++++++++++++++++++-------
>>>  1 file changed, 44 insertions(+), 7 deletions(-)
>> David, you being the maintainer of this code now, I don't think I've
>> seen a response from you on this patch, despite - iirc - Andrew
>> having pinged you on it already too.
>>
>> Jan
>>
> 
> David is out of the office for a week on vacation at the moment.
> 
> I did get code-review from him before submitting it upstream, but I
> guess that doesn't count for much as a formal ack.

Then why did you not include his Reviewed-by with the patch?
That would have sufficed (and put you in a bad light in case he
came back and said he didn't do any such review - which I trust
he did if you say so).

Jan


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