Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1356578

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

From Jianyu Zhan <nasa4836@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH 3/3] x86/irq: update first_system_vector only when X86_LOCAL_PIC is on
Date 2016-03-13 02:30 +0100
Message-ID <rc310-6Fl-7@gated-at.bofh.it> (permalink)
References <rbTl0-7Va-15@gated-at.bofh.it> <rbTuG-847-17@gated-at.bofh.it> <rbYb0-393-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Sun, Mar 13, 2016 at 4:08 AM, Thomas Gleixner <tglx@linutronix.de> 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
CONFIG_X86_LOCAL_APIC:

static void __init smp_intr_init(void)
{
#ifdef CONFIG_SMP
       /*
        * 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

#ifdef CONFIG_IRQ_WORK
       alloc_intr_gate(IRQ_WORK_VECTOR, irq_work_interrupt);
#endif

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)
>>        */
>>       i = FIRST_EXTERNAL_VECTOR;
>> -#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
to FIRST_SYSTEM_VECTOR:

int first_system_vector = FIRST_SYSTEM_VECTOR;

#ifdef CONFIG_X86_LOCAL_APIC
#define FIRST_SYSTEM_VECTOR             LOCAL_TIMER_VECTOR
#else
#define FIRST_SYSTEM_VECTOR             NR_VECTORS
#endif

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
!CONFIG_X86_LOCAL_APIC case.

Regards,
Jianyu Zhan

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/3] x86/irq: Refactor special vector definition and cleanup Jianyu Zhan <nasa4836@gmail.com> - 2016-03-12 16:10 +0100
  [PATCH 1/3] x86/asm/irq: Rearrange definitoin of specical irq vectors and cleanup. Jianyu Zhan <nasa4836@gmail.com> - 2016-03-12 16:10 +0100
  [PATCH 2/3] x86/irq: refactor native_init_IRQ Jianyu Zhan <nasa4836@gmail.com> - 2016-03-12 16:20 +0100
    Re: [PATCH 2/3] x86/irq: refactor native_init_IRQ kbuild test robot <lkp@intel.com> - 2016-03-12 16:40 +0100
      Re: [PATCH 2/3] x86/irq: refactor native_init_IRQ Jianyu Zhan <nasa4836@gmail.com> - 2016-03-12 16:40 +0100
    Re: [PATCH 2/3] x86/irq: refactor native_init_IRQ kbuild test robot <lkp@intel.com> - 2016-03-12 17:30 +0100
  [PATCH 3/3] x86/irq: update first_system_vector only when X86_LOCAL_PIC is on Jianyu Zhan <nasa4836@gmail.com> - 2016-03-12 16:20 +0100
    Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Thomas Gleixner <tglx@linutronix.de> - 2016-03-12 21:20 +0100
      Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Jianyu Zhan <nasa4836@gmail.com> - 2016-03-13 02:30 +0100
        Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Thomas Gleixner <tglx@linutronix.de> - 2016-03-13 07:40 +0100
          Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Jianyu Zhan <nasa4836@gmail.com> - 2016-03-13 08:30 +0100
            Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Jianyu Zhan <nasa4836@gmail.com> - 2016-03-13 08:50 +0100
            Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Thomas Gleixner <tglx@linutronix.de> - 2016-03-13 09:00 +0100
              Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Jianyu Zhan <nasa4836@gmail.com> - 2016-03-13 09:30 +0100
                Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Thomas Gleixner <tglx@linutronix.de> - 2016-03-13 10:20 +0100
                Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Jianyu Zhan <nasa4836@gmail.com> - 2016-03-13 10:40 +0100
                Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Thomas Gleixner <tglx@linutronix.de> - 2016-03-13 10:40 +0100
                Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Jianyu Zhan <nasa4836@gmail.com> - 2016-03-13 11:10 +0100
                Re: [PATCH 3/3] x86/irq: update first_system_vector only when  X86_LOCAL_PIC is on Thomas Gleixner <tglx@linutronix.de> - 2016-03-13 12:20 +0100

csiph-web