Re: [PATCH 3/3] x86/irq: update first_system_vector only when X86_LOCAL_PIC is on

From: Jianyu Zhan
Date: Sat Mar 12 2016 - 20:29:19 EST

On Sun, Mar 13, 2016 at 4:08 AM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> This is pointless, because it's only called when local apic is enabled as all
> call sites of alloc_intr_gate() depend on CONFIG_X86_LOCAL_APIC ....

Not exactly, currently at least smp_intr_init() DOES NOT depend on

static void __init smp_intr_init(void)
* The reschedule interrupt is a CPU-to-CPU reschedule-helper
* IPI, driven by wakeup.
alloc_intr_gate(RESCHEDULE_VECTOR, reschedule_interrupt);

/* IPI for generic function call */
alloc_intr_gate(CALL_FUNCTION_VECTOR, call_function_interrupt);


So alloc_intr_gate will be called, and first_system_vector will be updated !

I know this is weird, because modern SMP machines implies Local APIC.
But currently we have CONFIG_SMP detangle from CONFIG_X86_LOCAL_APIC,
which I think is fine.

Another place which is weird is CONFIG_IRQ_WORK. Technically, it
does not depend
on SMP, nor even necessary Local APIC. Actually, it is just a base
configuration selected
by others. But currently we have the

alloc_intr_gate(IRQ_WORK_VECTOR, irq_work_interrupt);

block surrounded by CONFIG_X86_LOCAL_APIC.

In new scheme, I just move it out, see [2/3] patch.

>> } else {
>> BUG();
>> }
>> diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
>> index 0e9fa7c..e999b38 100644
>> --- a/arch/x86/kernel/irqinit.c
>> +++ b/arch/x86/kernel/irqinit.c
>> @@ -188,9 +188,6 @@ void __init native_init_IRQ(void)
>> * 'special' SMP interrupts)
>> */
>> -#ifndef CONFIG_X86_LOCAL_APIC
>> -#define first_system_vector NR_VECTORS
>> -#endif
>> for_each_clear_bit_from(i, used_vectors, first_system_vector) {
> And how exactly is this here supposed to compile when CONFIG_X86_LOCAL_APIC=n?

Dunno. I guess this code on !CONFIG_X86_LOCAL_APIC case hasn't been
tested yet ?

first_system_vector is a global variable, and is initially assigned

int first_system_vector = FIRST_SYSTEM_VECTOR;


For CONFIG_X86_LOCAL_APIC case, the define makes sense.
But for ! CONFIG_X86_LOCAL_APIC case, why we confine it to NR_VECTORS
is a mystery
to me. Have digged into git history, but found no proof.

So to maintain consistency, this patch just retain what it is, but we
do not bother update it for

Jianyu Zhan