All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	Borislav Petkov <bp@alien8.de>, Toshi Kani <toshi.kani@hp.com>
Subject: Re: [PATCH] x86, mtrr: mark range_new in mtrr_calc_range_state() as __initdata
Date: Fri, 27 Nov 2015 10:31:25 +0100	[thread overview]
Message-ID: <20151127093125.GA28272@gmail.com> (raw)
In-Reply-To: <1447408073-25059-1-git-send-email-linux@rasmusvillemoes.dk>


* Rasmus Villemoes <linux@rasmusvillemoes.dk> 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 <linux@rasmusvillemoes.dk>
> ---
>  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?

Thanks,

	Ingo

  reply	other threads:[~2015-11-27  9:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-13  9:47 [PATCH] x86, mtrr: mark range_new in mtrr_calc_range_state() as __initdata Rasmus Villemoes
2015-11-27  9:31 ` Ingo Molnar [this message]
2015-12-01 11:58   ` Rasmus Villemoes
2015-12-01 16:14     ` Ingo Molnar
2015-12-01 20:44       ` [PATCH v2] " Rasmus Villemoes
2015-12-04 11:49         ` [tip:x86/mm] x86/mm/mtrr: Mark the 'range_new' static variable " tip-bot for Rasmus Villemoes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151127093125.GA28272@gmail.com \
    --to=mingo@kernel.org \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=toshi.kani@hp.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.