Messages in this thread | | | Date | Fri, 22 May 2009 22:33:32 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH] x86,APIC: Detect lapic_is_integrated() once - use on and on. |
| |
On Fri, 22 May 2009, Rakib Mullick wrote: > Impact: Reduce text space through efficient coding > > Determine apic is integrated or not - by calling > lapic_is_integrated() once from APIC_init_uniprocessor() and keep it > in a variable integrated_lapic. Thus we can determine apic is > integrated or not, by checking the variable instead of calling > lapic_is_integrated() on and on. Marking lapic_is_integrated() as > __init, which reduces some text space. Also allows us to get away from > messy #ifdef and inline. > > --- > Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com> > > --- linus/arch/x86/kernel/apic/apic.c 2009-05-21 22:22:15.000000000 +0600 > +++ rakib/arch/x86/kernel/apic/apic.c 2009-05-21 23:27:26.000000000 +0600 > @@ -127,6 +127,9 @@ early_param("nox2apic", setup_nox2apic); > > unsigned long mp_lapic_addr; > int disable_apic; > + > +static int integrated_lapic; > + > /* Disable local APIC timer from the kernel commandline or via dmi quirk */ > static int disable_apic_timer __cpuinitdata; > /* Local APIC timer works in C2 */ > @@ -188,13 +191,12 @@ static inline int lapic_get_version(void > /* > * Check, if the APIC is integrated or a separate chip > */ > -static inline int lapic_is_integrated(void) > +void __init lapic_is_integrated(void) > { > -#ifdef CONFIG_X86_64 > - return 1; > -#else > - return APIC_INTEGRATED(lapic_get_version()); > -#endif > + if (APIC_INTEGRATED(lapic_get_version())) > + integrated_lapic = 1; > + else > + integrated_lapic = 0; > }
I agree in general, but the patch is flawed as it prevents the compiler to optimize the X86_64 case out.
What you really want is:
#ifdef CONFIG_X86_64 # define integrated_lapic (1) #else static int integrated_lapic __read_mostly; #endif
static inline void lapic_is_integrated(void) { #ifdef CONFIG_X86_32 integrate_lapic = APIC_INTEGRATED(lapic_get_version()); #endif }
So that way the check is only done for 32bit systems and the compiler will optimize out the code which depends on !integrated_lapic.
Thanks,
tglx
| |