From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756029AbbLAL6r (ORCPT ); Tue, 1 Dec 2015 06:58:47 -0500 Received: from mail-lf0-f50.google.com ([209.85.215.50]:32933 "EHLO mail-lf0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754157AbbLAL6p (ORCPT ); Tue, 1 Dec 2015 06:58:45 -0500 From: Rasmus Villemoes To: Ingo Molnar Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, Borislav Petkov , Toshi Kani Subject: Re: [PATCH] x86, mtrr: mark range_new in mtrr_calc_range_state() as __initdata Organization: D03 References: <1447408073-25059-1-git-send-email-linux@rasmusvillemoes.dk> <20151127093125.GA28272@gmail.com> X-Hashcash: 1:20:151201:tglx@linutronix.de::0qGqodo70hFFB3pD:00000000000000000000000000000000000000000000+HB X-Hashcash: 1:20:151201:bp@alien8.de::1rxnQEyuOv1vpi8p:000000ky0 X-Hashcash: 1:20:151201:linux-kernel@vger.kernel.org::4tjFbfZ3iU7XrjgK:0000000000000000000000000000000002IZc X-Hashcash: 1:20:151201:toshi.kani@hp.com::HsfO0sAtzgAlwUTD:000000000000000000000000000000000000000000002Hcs X-Hashcash: 1:20:151201:hpa@zytor.com::KI/SvAtfpB/VnAXW:00002wAV X-Hashcash: 1:20:151201:x86@kernel.org::+p+ZwoAWVKcU/PS3:0003Kem X-Hashcash: 1:20:151201:mingo@kernel.org::oWv7bhna0cV3Ondi:04MLf X-Hashcash: 1:20:151201:mingo@redhat.com::wexYv34WZQ6lentb:0AqSb Date: Tue, 01 Dec 2015 12:58:42 +0100 In-Reply-To: <20151127093125.GA28272@gmail.com> (Ingo Molnar's message of "Fri, 27 Nov 2015 10:31:25 +0100") Message-ID: <87mvtubcst.fsf@rasmusvillemoes.dk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Nov 27 2015, Ingo Molnar wrote: > * Rasmus Villemoes wrote: > >> range_new doesn't seem to be used after init. It is only passed to >> memset, sum_ranges, memcmp and x86_get_mtrr_mem_range, the latter of >> which also only passes it on to various *range* library functions. So >> mark it __initdata to free up an extra page after init. >> >> nr_range_new is unconditionally assigned to before it is read, so >> there's no point in having it static. >> >> Signed-off-by: Rasmus Villemoes >> --- >> arch/x86/kernel/cpu/mtrr/cleanup.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kernel/cpu/mtrr/cleanup.c b/arch/x86/kernel/cpu/mtrr/cleanup.c >> index 70d7c93f4550..b1a9ad366f67 100644 >> --- a/arch/x86/kernel/cpu/mtrr/cleanup.c >> +++ b/arch/x86/kernel/cpu/mtrr/cleanup.c >> @@ -593,9 +593,9 @@ mtrr_calc_range_state(u64 chunk_size, u64 gran_size, >> unsigned long x_remove_base, >> unsigned long x_remove_size, int i) >> { >> - static struct range range_new[RANGE_NUM]; >> + static struct range range_new[RANGE_NUM] __initdata; >> unsigned long range_sums_new; >> - static int nr_range_new; >> + int nr_range_new; >> int num_reg; >> >> /* Convert ranges to var ranges state: */ > > So this static variable actually surprised me - I never realized it was there - > and it's not some simple 'once' flag, but something that is essential semantics. > > So marking it __initdata is correct, but please also move it out of function local > variables scope, into file scope - and name it properly as well, like > mtrr_new_range[] or so? I can certainly do that, but isn't the usual preference to keep the scope as small as possible? IOW, why do you want to make this a file-scoped variable? Also, I don't really see how the 'static' has 'essential semantics'. AFAICT, the contents are wiped on every invocation of mtrr_calc_range_state, so the only reason it's static is to avoid blowing the stack. Rasmus