Re: x86: adjust handling of interrupts coming in via legacy vectors

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 15/05/12 09:22, Jan Beulich wrote:
>>>> On 15.05.12 at 10:03, AP <apxeng@gmail.com> wrote:
>> On Mon, May 14, 2012 at 11:43 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 14.05.12 at 18:24, Keir Fraser <keir@xen.org> wrote:
>>>> On 14/05/2012 16:56, "Jan Beulich" <JBeulich@suse.com> wrote:
>>>>
>>>>>> Looks sensible, and I suppose good to have for 4.2.
>>>>>>
>>>>>> Acked-by: Keir Fraser <keir@xen.org>
>>>>> Please take a look at the v2 I just sent, to accommodate a suggestion
>>>>> from Andrew Cooper.
>>>> I think it's very paranoid, since legacy vectors never get programmed
>>>> into
>>>> an IOAPIC RTE and should never need EOIing at the local APIC. But you do
>>>> at
>>>> least printk the case that we see the ISR bit set, and you printk the
>>>> vector
>>>> number, so really this v2 patch gives us more information about this
>>>> bogus
>>>> situation than v1 did, so it's a slight improvement overall. So you
>>>> still
>>>> have my Ack.
>>> It indeed is paranoid (which is why I didn't do so in v1), but Andrew
>>> certainly has a point in saying that something so far unexplainable
>>> going on makes it desirable to cover as many (however remotely)
>>> potential causes as possible. (I still consider double delivery through
>>> IO-APIC and PIC the most likely scenario, despite not having a
>>> reasonably explanation on how the PIC mask bit could get cleared.)
>>>
>>> Once we hopefully understand the hole situation, the code here
>>> should likely be reverted to the v1 version (along with removing the
>>> other debugging code).
>> Once this patch goes in, do I need to still run with the patch Andrew
>> provided in http://lists.xen.org/archives/html/xen-devel/2012-05/msg00332.html 
>> for the debugging code?
> Yes, that change is still going to be necessary. Probably worth
> committing too (perhaps with its second hunk annotated with a
> comment), which I believe didn't happen because it wasn't really
> submitted for that purpose. Andrew, Keir?
>
> Or would we be better off simply allowing xfree(NULL) in IRQ
> context, by swapping the in_irq() and NULL checks in the
> function)? I'd favor this, despite the small risk of it hiding
> latent bugs.
>
> Jan

The patch should probably be committed in the same vein as the other
debugging patches

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com