From: jszhang@marvell.com (Jisheng Zhang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 3/4] arm64: kaslr: Fix incorrect placement of __initdata and __read_mostly
Date: Mon, 15 Aug 2016 12:52:14 +0800 [thread overview]
Message-ID: <20160815125214.2727bf88@xhacker> (raw)
In-Reply-To: <CAKv+Gu_ci37u6av9gjchBTVwoMCiqdnwtnDHxLT3cV3+kG0wUw@mail.gmail.com>
Hi Ard,
On Fri, 12 Aug 2016 14:02:40 +0200 Ard Biesheuvel wrote:
> Hi Jisheng,
>
> On 12 August 2016 at 10:01, Jisheng Zhang <jszhang@marvell.com> wrote:
> > __initdata and __read_mostly should be placed after the variable name
> > for the variable to be placed in the intended section.
> >
>
> Why?
include/linux/init.h says something as:
* For initialized data:
* You should insert __initdata or __initconst between the variable name
* and equal sign followed by value, e.g.:
*
* static int init_variable __initdata = 0;
* static const char linux_logo[] __initconst = { 0x32, 0x36, ... };
and examples in gcc manual also put __attribute__ (...) after variable name.
https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes
Then I grep the source, found most lines (especially arch/arm64/*) put the
__initdata and __read_mostly after the variable name.
However, I built the code with three different gcc, the result looks identical
no matter where these markers put. So the commit msg looks wrong, what about
changes it as
"put __initdata and __read_mostly after the variable name
to keep the style consistent"?
Thanks,
Jisheng
>
> > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> > ---
> > arch/arm64/kernel/kaslr.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> > index b054691..8ebabc4 100644
> > --- a/arch/arm64/kernel/kaslr.c
> > +++ b/arch/arm64/kernel/kaslr.c
> > @@ -20,8 +20,8 @@
> > #include <asm/pgtable.h>
> > #include <asm/sections.h>
> >
> > -u64 __read_mostly module_alloc_base;
> > -u16 __initdata memstart_offset_seed;
> > +u64 module_alloc_base __read_mostly;
> > +u16 memstart_offset_seed __initdata;
> >
> > static __init u64 get_kaslr_seed(void *fdt)
> > {
> > --
> > 2.8.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Jisheng Zhang <jszhang@marvell.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
"Kees Cook" <keescook@chromium.org>,
Mark Rutland <mark.rutland@arm.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3 3/4] arm64: kaslr: Fix incorrect placement of __initdata and __read_mostly
Date: Mon, 15 Aug 2016 12:52:14 +0800 [thread overview]
Message-ID: <20160815125214.2727bf88@xhacker> (raw)
In-Reply-To: <CAKv+Gu_ci37u6av9gjchBTVwoMCiqdnwtnDHxLT3cV3+kG0wUw@mail.gmail.com>
Hi Ard,
On Fri, 12 Aug 2016 14:02:40 +0200 Ard Biesheuvel wrote:
> Hi Jisheng,
>
> On 12 August 2016 at 10:01, Jisheng Zhang <jszhang@marvell.com> wrote:
> > __initdata and __read_mostly should be placed after the variable name
> > for the variable to be placed in the intended section.
> >
>
> Why?
include/linux/init.h says something as:
* For initialized data:
* You should insert __initdata or __initconst between the variable name
* and equal sign followed by value, e.g.:
*
* static int init_variable __initdata = 0;
* static const char linux_logo[] __initconst = { 0x32, 0x36, ... };
and examples in gcc manual also put __attribute__ (...) after variable name.
https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes
Then I grep the source, found most lines (especially arch/arm64/*) put the
__initdata and __read_mostly after the variable name.
However, I built the code with three different gcc, the result looks identical
no matter where these markers put. So the commit msg looks wrong, what about
changes it as
"put __initdata and __read_mostly after the variable name
to keep the style consistent"?
Thanks,
Jisheng
>
> > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> > ---
> > arch/arm64/kernel/kaslr.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> > index b054691..8ebabc4 100644
> > --- a/arch/arm64/kernel/kaslr.c
> > +++ b/arch/arm64/kernel/kaslr.c
> > @@ -20,8 +20,8 @@
> > #include <asm/pgtable.h>
> > #include <asm/sections.h>
> >
> > -u64 __read_mostly module_alloc_base;
> > -u16 __initdata memstart_offset_seed;
> > +u64 module_alloc_base __read_mostly;
> > +u16 memstart_offset_seed __initdata;
> >
> > static __init u64 get_kaslr_seed(void *fdt)
> > {
> > --
> > 2.8.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2016-08-15 4:52 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-12 8:01 [PATCH v3 0/4] arm64: put objects into proper sections Jisheng Zhang
2016-08-12 8:01 ` Jisheng Zhang
2016-08-12 8:01 ` [PATCH v3 1/4] arm64: vdso: add __init section marker to alloc_vectors_page Jisheng Zhang
2016-08-12 8:01 ` Jisheng Zhang
2016-08-12 8:01 ` [PATCH v3 2/4] arm64: vdso: constify vm_special_mapping used for aarch32 vectors page Jisheng Zhang
2016-08-12 8:01 ` Jisheng Zhang
2016-08-12 8:01 ` [PATCH v3 3/4] arm64: kaslr: Fix incorrect placement of __initdata and __read_mostly Jisheng Zhang
2016-08-12 8:01 ` Jisheng Zhang
2016-08-12 12:02 ` Ard Biesheuvel
2016-08-12 12:02 ` Ard Biesheuvel
2016-08-15 4:52 ` Jisheng Zhang [this message]
2016-08-15 4:52 ` Jisheng Zhang
2016-08-15 5:57 ` Jisheng Zhang
2016-08-15 5:57 ` Jisheng Zhang
2016-08-15 6:30 ` Ard Biesheuvel
2016-08-15 6:30 ` Ard Biesheuvel
2016-08-12 8:01 ` [PATCH v3 4/4] arm64: apply __ro_after_init to some objects Jisheng Zhang
2016-08-12 8:01 ` Jisheng Zhang
2016-08-12 12:43 ` Mark Rutland
2016-08-12 12:43 ` Mark Rutland
2016-08-15 6:53 ` Jisheng Zhang
2016-08-15 6:53 ` Jisheng Zhang
2016-08-15 9:07 ` Mark Rutland
2016-08-15 9:07 ` Mark Rutland
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=20160815125214.2727bf88@xhacker \
--to=jszhang@marvell.com \
--cc=linux-arm-kernel@lists.infradead.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.