* Dou Liyang <douly.f...@cn.fujitsu.com> wrote:

> the pending interrupt check code is mixed with the local APIC setup code,
> that looks messy.
> 
> Extract the related code, move it into a new function named
> apic_pending_intr_clear().
> 
> bonus cleanups from Andy Shevchenko's suggestions:
> 
>   - for() -> for_each_set_bit()
>   - printk() -> pr_err()

Please split the cleanups (and the cleanups suggested further below) into a 
separate patch, so that there's a pure 'code movement' patch plus another patch 
that is easy to review.

> +     /*
> +      * After a crash, we no longer service the interrupts and a pending
> +      * interrupt from previous kernel might still have ISR bit set.
> +      *
> +      * Most probably by now CPU has serviced that pending interrupt and
> +      * it might not have done the ack_APIC_irq() because it thought,
> +      * interrupt came from i8259 as ExtInt. LAPIC did not get EOI so it
> +      * does not clear the ISR bit and cpu thinks it has already serivced
> +      * the interrupt. Hence a vector might get locked. It was noticed
> +      * for timer irq (vector 0x31). Issue an extra EOI to clear ISR.
> +      */
> +     do {
> +             queued = 0;
> +             for (i = APIC_ISR_NR - 1; i >= 0; i--)
> +                     queued |= apic_read(APIC_IRR + i*0x10);
> +
> +             for (i = APIC_ISR_NR - 1; i >= 0; i--) {
> +                     value = apic_read(APIC_ISR + i*0x10);
> +                     for_each_set_bit(j, &value, 32) {
> +                             if (j) {
> +                                     ack_APIC_irq();
> +                                     acked++;
> +                             }
> +                     }
> +             }
> +             if (acked > 256) {
> +                     pr_err("LAPIC pending interrupts after %d EOI\n",
> +                             acked);

Please don't
        break the line of
        printk's.


> +             if (queued) {
> +                     if (boot_cpu_has(X86_FEATURE_TSC) && cpu_khz) {
> +                             ntsc = rdtsc();
> +                             max_loops = (cpu_khz << 10) - (ntsc - tsc);
> +                     } else
> +                             max_loops--;

unbalanced curly braces.

Thanks,

        Ingo

Reply via email to