From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757887AbZEVUIm (ORCPT ); Fri, 22 May 2009 16:08:42 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1757195AbZEVUIf (ORCPT ); Fri, 22 May 2009 16:08:35 -0400 Received: from fg-out-1718.google.com ([72.14.220.159]:5192 "EHLO fg-out-1718.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756862AbZEVUIe (ORCPT ); Fri, 22 May 2009 16:08:34 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=E+6/yEyaC7oSz6Ink4EkINFNKh53sHXJ3Z1K53BU6LQWiSH2lJBDD/H1hu/zjtS0Ax n8g28mDiWVrFWgsFcSK+IwMCptCFH07czPvzwJYXF2Qg/Gq3/5M/psgZtFLT5esib5ra CdGqc7o3n/u70Hx0IJmYkjFcs8nU30XjbUhTA= Date: Sat, 23 May 2009 00:02:39 +0400 From: Cyrill Gorcunov To: Rakib Mullick Cc: Ingo Molnar , akpm@linux-foundation.org, linux-kernel@vger.kernel.org, "H. Peter Anvin" , Yinghai Lu , "Maciej W. Rozycki" Subject: Re: [PATCH] x86,APIC: Detect lapic_is_integrated() once - use on and on. Message-ID: <20090522200239.GD5354@lenovo> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org [Rakib Mullick - Fri, May 22, 2009 at 07:31:29PM +0600] | 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 | | --- 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; | } Hi Rakib, actually this change could be dangerous. I don't remember if I saw mixed configuration at all but I would not be that sure that we will never met it. Peter? Ingo? Yinghai? Maciej? | | /* | @@ -258,7 +260,7 @@ void __cpuinit enable_NMI_through_LVT0(v | v = APIC_DM_NMI; | | /* Level triggered for 82489DX (32bit mode) */ | - if (!lapic_is_integrated()) | + if (!integrated_lapic) | v |= APIC_LVT_LEVEL_TRIGGER; | | apic_write(APIC_LVT0, v); | @@ -313,7 +315,7 @@ static void __setup_APIC_LVTT(unsigned i | lvtt_value = LOCAL_TIMER_VECTOR; | if (!oneshot) | lvtt_value |= APIC_LVT_TIMER_PERIODIC; | - if (!lapic_is_integrated()) | + if (!integrated_lapic) | lvtt_value |= SET_APIC_TIMER_BASE(APIC_TIMER_BASE_DIV); | | if (!irqen) | @@ -869,7 +871,7 @@ void clear_local_APIC(void) | apic_write(APIC_LVTPC, APIC_LVT_MASKED); | | /* Integrated APIC (!82489DX) ? */ | - if (lapic_is_integrated()) { | + if (integrated_lapic) { | if (maxlvt > 3) | /* Clear ESR due to Pentium errata 3AP and 11AP */ | apic_write(APIC_ESR, 0); | @@ -1064,7 +1066,7 @@ void __init init_bsp_APIC(void) | */ | apic_write(APIC_LVT0, APIC_DM_EXTINT); | value = APIC_DM_NMI; | - if (!lapic_is_integrated()) /* 82489DX */ | + if (!integrated_lapic) /* 82489DX */ | value |= APIC_LVT_LEVEL_TRIGGER; | apic_write(APIC_LVT1, value); | } | @@ -1073,7 +1075,7 @@ static void __cpuinit lapic_setup_esr(vo | { | unsigned int oldvalue, value, maxlvt; | | - if (!lapic_is_integrated()) { | + if (!integrated_lapic) { | pr_info("No ESR for 82489DX.\n"); | return; | } | @@ -1126,7 +1128,7 @@ void __cpuinit setup_local_APIC(void) | | #ifdef CONFIG_X86_32 | /* Pound the ESR really hard over the head with a big hammer - mbligh */ | - if (lapic_is_integrated() && apic->disable_esr) { | + if (integrated_lapic && apic->disable_esr) { | apic_write(APIC_ESR, 0); | apic_write(APIC_ESR, 0); | apic_write(APIC_ESR, 0); | @@ -1251,7 +1253,7 @@ void __cpuinit setup_local_APIC(void) | value = APIC_DM_NMI; | else | value = APIC_DM_NMI | APIC_LVT_MASKED; | - if (!lapic_is_integrated()) /* 82489DX */ | + if (!integrated_lapic) /* 82489DX */ | value |= APIC_LVT_LEVEL_TRIGGER; | apic_write(APIC_LVT1, value); | | @@ -1585,6 +1587,8 @@ int __init APIC_init_uniprocessor(void) | pr_info("Apic disabled by BIOS\n"); | return -1; | } | + /* Since CONFIG_X86_64, so it's integrated. */ | + integrated_lapic = 1; | #else | if (!smp_found_config && !cpu_has_apic) | return -1; | @@ -1599,6 +1603,8 @@ int __init APIC_init_uniprocessor(void) | clear_cpu_cap(&boot_cpu_data, X86_FEATURE_APIC); | return -1; | } | + /* Determine APIC is integrated or not */ | + lapic_is_integrated(); | #endif | | enable_IR_x2apic(); | -- Cyrill