Re: [patch 00/21] 2.6.19-stable review

Zwane Mwaikambo
2007-02-28T08:51:24Z

On Tue, 27 Feb 2007, Eric W. Biederman wrote:

> > http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20/2.6.20-mm2/broken-out/x86_64-mm-handle-irqs-pending-in-irr-during-irq-migration.patch
> >>
> >> That's mainly an Andi decision. Let's cc him.
> >
> > Would be good to have Eric also ack them as safe and obvious.
> >
> > Btw, that latter one has corrupted sign-offs from Andi (it's in the middle
> > of the text, very confusing).
>
> There are two questions.
> 1) What can we do to make the situation better.
> 2) Is the hole completely plugged.
>
> When I wrote the patch I had the local apic priorities backwards in my
> head. So apic_in_service_vector can return the wrong value if two
> irqs are in service. Now I don't think we allows ourselves to enable
> interrupts in an interrupt service routing until after we have acked
> the local apic so this should be harmless. The fix is also trivial
> of just having apic_in_service_vector return: "~get_irq_regs()->orig_rax".
>
> Except for that one possible problem everything I can think of are
> just theoretical cracks at this point, and they don't make the
> situation any worse.
>
> Given that this patch has appears to have undergone a noticeable
> amount of testing, by people other than myself, and clears up the
> symptoms. I have no problem

Hi Eric,
Thanks for getting this cruft cleaned up. I have a few comments
regarding;

handle-irqs-pending-in-irr-during-irq-migration.patch

1) It relies on checking the IRR, this could race with the corresponding
vector bit being set by hardware.

2) apic_handle_pending_vector is oddly named since it doesn't actually
handle a pending vector but drops it if it has been freed.

3) It looks complex

So how about the following scheme. Add a check in do_IRQ whether the irq's
domain contains the current cpu, if not we free the vector upon handler
completion.

Cheers,
Zwane