Messages in this thread | | | Subject | Re: [RFC][PATCH] kdump: x86_64 timer interrupt lockup due to pending interrupt | From | (Eric W. Biederman) | Date | Tue, 07 Mar 2006 21:04:47 -0700 |
| |
Vivek Goyal <vgoyal@in.ibm.com> writes:
> On Tue, Mar 07, 2006 at 04:43:07PM -0700, Eric W. Biederman wrote: >> Vivek Goyal <vgoyal@in.ibm.com> writes: >> > On Mon, Mar 06, 2006 at 10:43:32PM +0100, Andi Kleen wrote: >> >> On Mon, Mar 06, 2006 at 11:40:34AM -0500, Vivek Goyal wrote: >> >> > > > [..] >> >> > >> >> > o In this patch, one extra EOI is being issued in check_timer() to unlock >> > the >> >> > vector. Please suggest if there is a better way to handle this situation. >> >> >> >> Shouldn't we rather do this for all interrupts when the APIC is set up? >> >> I don't see how the timer is special here. >> >> >> > >> > Timer is a special case here. >> > >> > In other cases, the moment interrupts are enabled on cpu, LAPIC pushes > pending >> > interrupts to cpu and it is ignored as bad irq using ack_bad_irq(). This >> > still sends EOI to LAPIC if LPAIC support is compiled in. >> > >> > But for timer, the moment pending interrupt is pushed to cpu, it is handled >> > as valid interrupt and cpu assumes that it came from 8259 and sends ack to >> > 8259 and not to LAPIC. Hence leads to missing EOI for timer vector and >> > deadlock. >> > >> > But still doing it generic manner for all interrupts while setting up LAPIC >> > probably makes more sense. Please find attached the patch. >> >> A couple of questions. >> >> Does this need to be in #ifdef CONFIG_CRASS_DUMP? >> If this code is truly safe I expect we could run it on every bootup >> simply to be more robust. >> > > AFAIK, we can run this code safely on every bootup and can get rid of > CONFIG_CRASH_DUMP. I have simply put it under it because I observed it > only for crashdump scenarios. But removing this should be good as it > protectets agains buggy boards. Modified patch is attached. > > >> Why is APIC_ISR_NR a hard code? I think there is an apic register >> that tells the count. >> > > I did not find any such register. Basically ISR is a 256bit register. We > are reading 32 bits at a time, so logically we can view it as 8, 32 bit > registers. I had two options. Either I put a constant number in for() > loop or #define it. I chose later one. > >> Does ack_APIC_irq take an argument? I am confused that we are calling >> ack_APIC_irq() potentially 8*32 times without passing it anything. >> > > It does not take any argument. Whenever a zero is written to EOI register > LAPIC resets one ISR register bit corresponding to highest priority > interrupt. So if all the ISR bits are set, we will call ack_APIC_irq() > 8*32 times to reset them all.
Ok. That makes sense.
Looks good to me.
Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |