Re: [PATCH v2] x86/apic: Move pending intr check code into it's own function

[Ingo Molnar <mingo@kernel.org>]

* Dou Liyang <douly.fnst@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