From: Dave Hansen <dave.hansen@intel.com>
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
Andy Lutomirski <luto@kernel.org>,
Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Nathan Chancellor <natechancellor@gmail.com>,
Nick Desaulniers <ndesaulniers@google.com>,
linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com,
kernel-janitors@vger.kernel.org, linux-safety@lists.elisa.tech
Subject: Re: [PATCH] x86/mm: drop superfluous initialization
Date: Mon, 28 Sep 2020 10:59:36 -0700 [thread overview]
Message-ID: <1d376c1f-2f14-5c29-671e-ca59853fa4a4@intel.com> (raw)
In-Reply-To: <20200928100004.25674-1-lukas.bulwahn@gmail.com>
On 9/28/20 3:00 AM, Lukas Bulwahn wrote:
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index c7a47603537f..5632f02146ca 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -609,7 +609,7 @@ static void __init memory_map_top_down(unsigned long map_start,
> step_size = PMD_SIZE;
> max_pfn_mapped = 0; /* will get exact value next */
> min_pfn_mapped = real_end >> PAGE_SHIFT;
> - last_start = start = real_end;
> + last_start = real_end;
Thanks for finding this.
This becomes even more obviously correct if we just move the 'start'
declaration into the while() loop. If we do that, it puts the three
assignment locations right next to the definition, and its trivial to
spot that the initialization was not missed:
while (last_start > map_start) {
unsigned long start;
if (last_start > step_size) {
start = round_down(last_start - 1, step_size);
if (start < map_start)
start = map_start;
} else
start = map_start;
...
Either way, your patch looks correct to me:
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <dave.hansen@intel.com>
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
Andy Lutomirski <luto@kernel.org>,
Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Nathan Chancellor <natechancellor@gmail.com>,
Nick Desaulniers <ndesaulniers@google.com>,
linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com,
kernel-janitors@vger.kernel.org, linux-safety@lists.elisa.tech
Subject: Re: [PATCH] x86/mm: drop superfluous initialization
Date: Mon, 28 Sep 2020 17:59:36 +0000 [thread overview]
Message-ID: <1d376c1f-2f14-5c29-671e-ca59853fa4a4@intel.com> (raw)
In-Reply-To: <20200928100004.25674-1-lukas.bulwahn@gmail.com>
On 9/28/20 3:00 AM, Lukas Bulwahn wrote:
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index c7a47603537f..5632f02146ca 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -609,7 +609,7 @@ static void __init memory_map_top_down(unsigned long map_start,
> step_size = PMD_SIZE;
> max_pfn_mapped = 0; /* will get exact value next */
> min_pfn_mapped = real_end >> PAGE_SHIFT;
> - last_start = start = real_end;
> + last_start = real_end;
Thanks for finding this.
This becomes even more obviously correct if we just move the 'start'
declaration into the while() loop. If we do that, it puts the three
assignment locations right next to the definition, and its trivial to
spot that the initialization was not missed:
while (last_start > map_start) {
unsigned long start;
if (last_start > step_size) {
start = round_down(last_start - 1, step_size);
if (start < map_start)
start = map_start;
} else
start = map_start;
...
Either way, your patch looks correct to me:
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
next prev parent reply other threads:[~2020-09-28 17:59 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-28 10:00 [PATCH] x86/mm: drop superfluous initialization Lukas Bulwahn
2020-09-28 10:00 ` Lukas Bulwahn
2020-09-28 17:59 ` Dave Hansen [this message]
2020-09-28 17:59 ` Dave Hansen
2020-09-29 8:42 ` Lukas Bulwahn
2020-09-29 8:42 ` Lukas Bulwahn
2020-09-29 8:42 ` Lukas Bulwahn
2020-11-20 11:51 ` [tip: x86/cleanups] x86/mm: Declare 'start' variable where it is used tip-bot2 for Lukas Bulwahn
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=1d376c1f-2f14-5c29-671e-ca59853fa4a4@intel.com \
--to=dave.hansen@intel.com \
--cc=bp@alien8.de \
--cc=clang-built-linux@googlegroups.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-safety@lists.elisa.tech \
--cc=lukas.bulwahn@gmail.com \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=natechancellor@gmail.com \
--cc=ndesaulniers@google.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--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.