| Message ID | 20190812215052.71840-12-ndesaulniers@google.com |
|---|---|
| State | Accepted |
| Commit | 80d838122643a09a9f99824adea4b4261e4451e6 |
| Headers | show |
| Series | [01/16] s390/boot: fix section name escaping | expand |
Hi Nick, On Mon, Aug 12, 2019 at 02:50:45PM -0700, Nick Desaulniers wrote: > GCC unescapes escaped string section names while Clang does not. Because > __section uses the `#` stringification operator for the section name, it > doesn't need to be escaped. > > This antipattern was found with: > $ grep -e __section\(\" -e __section__\(\" -r > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > arch/arm64/include/asm/cache.h | 2 +- > arch/arm64/kernel/smp_spin_table.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Does this fix a build issue, or is it just cosmetic or do we end up with duplicate sections or something else? Happy to route it via arm64, just having trouble working out whether it's 5.3 material! Will
On Tue, Aug 13, 2019 at 10:27 AM Will Deacon <will@kernel.org> wrote: > > Hi Nick, > > On Mon, Aug 12, 2019 at 02:50:45PM -0700, Nick Desaulniers wrote: > > GCC unescapes escaped string section names while Clang does not. Because > > __section uses the `#` stringification operator for the section name, it > > doesn't need to be escaped. > > > > This antipattern was found with: > > $ grep -e __section\(\" -e __section__\(\" -r > > > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > --- > > arch/arm64/include/asm/cache.h | 2 +- > > arch/arm64/kernel/smp_spin_table.c | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > Does this fix a build issue, or is it just cosmetic or do we end up with > duplicate sections or something else? This should be cosmetic -- basically we are trying to move all users of current available __attribute__s in compiler_attributes.h to the __attr forms. I am also adding (slowly) new attributes that are already used but we don't have them yet in __attr form. > Happy to route it via arm64, just having trouble working out whether it's > 5.3 material! As you prefer! Those that are not taken by a maintainer I will pick up and send via compiler-attributes. I would go for 5.4, since there is no particular rush anyway. Cheers, Miguel
On Tue, Aug 13, 2019 at 02:36:06PM +0200, Miguel Ojeda wrote: > On Tue, Aug 13, 2019 at 10:27 AM Will Deacon <will@kernel.org> wrote: > > On Mon, Aug 12, 2019 at 02:50:45PM -0700, Nick Desaulniers wrote: > > > GCC unescapes escaped string section names while Clang does not. Because > > > __section uses the `#` stringification operator for the section name, it > > > doesn't need to be escaped. > > > > > > This antipattern was found with: > > > $ grep -e __section\(\" -e __section__\(\" -r > > > > > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > > > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > > --- > > > arch/arm64/include/asm/cache.h | 2 +- > > > arch/arm64/kernel/smp_spin_table.c | 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > Does this fix a build issue, or is it just cosmetic or do we end up with > > duplicate sections or something else? > > This should be cosmetic -- basically we are trying to move all users > of current available __attribute__s in compiler_attributes.h to the > __attr forms. I am also adding (slowly) new attributes that are > already used but we don't have them yet in __attr form. > > > Happy to route it via arm64, just having trouble working out whether it's > > 5.3 material! > > As you prefer! Those that are not taken by a maintainer I will pick up > and send via compiler-attributes. > > I would go for 5.4, since there is no particular rush anyway. Okey doke, I'll pick this one up for 5.4 then. Thanks for the explanation! Will
On Tue, Aug 13, 2019 at 10:08 AM Will Deacon <will@kernel.org> wrote: > > On Tue, Aug 13, 2019 at 02:36:06PM +0200, Miguel Ojeda wrote: > > On Tue, Aug 13, 2019 at 10:27 AM Will Deacon <will@kernel.org> wrote: > > > On Mon, Aug 12, 2019 at 02:50:45PM -0700, Nick Desaulniers wrote: > > > > GCC unescapes escaped string section names while Clang does not. Because > > > > __section uses the `#` stringification operator for the section name, it > > > > doesn't need to be escaped. > > > > > > > > This antipattern was found with: > > > > $ grep -e __section\(\" -e __section__\(\" -r > > > > > > > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> > > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > > > --- > > > > arch/arm64/include/asm/cache.h | 2 +- > > > > arch/arm64/kernel/smp_spin_table.c | 2 +- > > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > Does this fix a build issue, or is it just cosmetic or do we end up with > > > duplicate sections or something else? > > > > This should be cosmetic -- basically we are trying to move all users > > of current available __attribute__s in compiler_attributes.h to the > > __attr forms. I am also adding (slowly) new attributes that are > > already used but we don't have them yet in __attr form. This lone patch of the series is just cosmetic, but patch 14/16 fixes a real boot issue: https://github.com/ClangBuiltLinux/linux/issues/619 Miguel, I'd like to get that one landed ASAP; the rest are just for consistency. > > > > > Happy to route it via arm64, just having trouble working out whether it's > > > 5.3 material! Thanks! https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?h=for-next/core&id=80d838122643a09a9f99824adea4b4261e4451e6 > > > > As you prefer! Those that are not taken by a maintainer I will pick up > > and send via compiler-attributes. Miguel, how do you want to take the rest of these patches? Will picked up the arm64 one, I think the SuperH one got picked up. There was feedback to add more info to individual commits' commit messages. I kept these tree wide changes separate to improve the likelihood that they'd backport to stable cleanly, but could always squash if you'd prefer to have 1 patch instead of a series. Just let me know. -- Thanks, ~Nick Desaulniers
On Thu, Aug 15, 2019 at 12:20 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > This lone patch of the series is just cosmetic, but patch 14/16 fixes > a real boot issue: > https://github.com/ClangBuiltLinux/linux/issues/619 > Miguel, I'd like to get that one landed ASAP; the rest are just for consistency. Ah, interesting. It would be best to have sent that one independently to the others, plus adding a commit message mentioning this in particular. Let's talk about that in the thread. > Miguel, how do you want to take the rest of these patches? Will picked > up the arm64 one, I think the SuperH one got picked up. There was > feedback to add more info to individual commits' commit messages. Yes, I told Will I would pick up whatever is not already picked up by individual maintainers. > I kept these tree wide changes separate to improve the likelihood that > they'd backport to stable cleanly, but could always squash if you'd > prefer to have 1 patch instead of a series. Just let me know. Since you already did the splitting work, let's take advantage of it. I prefer them to be split anyway, since that gives maintainers a chance to pick them up individually if they prefer to do so. Cheers, Miguel
On Thu, Aug 15, 2019 at 11:08 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Thu, Aug 15, 2019 at 12:20 AM Nick Desaulniers > <ndesaulniers@google.com> wrote: > > > > This lone patch of the series is just cosmetic, but patch 14/16 fixes > > a real boot issue: > > https://github.com/ClangBuiltLinux/linux/issues/619 > > Miguel, I'd like to get that one landed ASAP; the rest are just for consistency. > > Ah, interesting. It would be best to have sent that one independently > to the others, plus adding a commit message mentioning this in > particular. Let's talk about that in the thread. Btw, I guess that is the Oops you were mentioning in the cover letter? Cheers, Miguel
On Thu, Aug 15, 2019 at 11:12 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > Btw, I guess that is the Oops you were mentioning in the cover letter? Pinging about this... Cheers, Miguel
On Sat, Aug 24, 2019 at 1:25 PM Will Deacon <will@kernel.org> wrote: > > Which bit are you pinging about? This patch (12/16) has been in -next for a > while and is queued in the arm64 tree for 5.4. The Oops/boot issue is > addressed in patch 14 which probably needs to be sent as a separate patch > (with a commit message) if it's targetting 5.3 and, I assume, routed via > somebody like akpm. I was pinging about the bit I was quoting, i.e. whether the Oops in the cover letter was #14 indeed. Also, since Nick said he wanted to get this ASAP through compiler-attributes, I assumed he wanted it to be in 5.3, but I have not seen the independent patch. Since he seems busy, I will write a better commit message myself and send it to Linus next week. Cheers, Miguel
On Sat, Aug 24, 2019 at 5:48 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Sat, Aug 24, 2019 at 1:25 PM Will Deacon <will@kernel.org> wrote: > > > > Which bit are you pinging about? This patch (12/16) has been in -next for a > > while and is queued in the arm64 tree for 5.4. The Oops/boot issue is > > addressed in patch 14 which probably needs to be sent as a separate patch > > (with a commit message) if it's targetting 5.3 and, I assume, routed via > > somebody like akpm. > > I was pinging about the bit I was quoting, i.e. whether the Oops in > the cover letter was #14 indeed. Also, since Nick said he wanted to > get this ASAP through compiler-attributes, I assumed he wanted it to > be in 5.3, but I have not seen the independent patch. > > Since he seems busy, I will write a better commit message myself and > send it to Linus next week. Sorry, very hectic week here last week. I'll try to get the import bit split off, collect the acks/reviewed-by tags, and resend a v2 of the series this week. -- Thanks, ~Nick Desaulniers
diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h index 64eeaa41e7ca..43da6dd29592 100644 --- a/arch/arm64/include/asm/cache.h +++ b/arch/arm64/include/asm/cache.h @@ -78,7 +78,7 @@ static inline u32 cache_type_cwg(void) return (read_cpuid_cachetype() >> CTR_CWG_SHIFT) & CTR_CWG_MASK; } -#define __read_mostly __attribute__((__section__(".data..read_mostly"))) +#define __read_mostly __section(.data..read_mostly) static inline int cache_line_size_of_cpu(void) { diff --git a/arch/arm64/kernel/smp_spin_table.c b/arch/arm64/kernel/smp_spin_table.c index 76c2739ba8a4..c8a3fee00c11 100644 --- a/arch/arm64/kernel/smp_spin_table.c +++ b/arch/arm64/kernel/smp_spin_table.c @@ -19,7 +19,7 @@ #include <asm/smp_plat.h> extern void secondary_holding_pen(void); -volatile unsigned long __section(".mmuoff.data.read") +volatile unsigned long __section(.mmuoff.data.read) secondary_holding_pen_release = INVALID_HWID; static phys_addr_t cpu_release_addr[NR_CPUS];
GCC unescapes escaped string section names while Clang does not. Because __section uses the `#` stringification operator for the section name, it doesn't need to be escaped. This antipattern was found with: $ grep -e __section\(\" -e __section__\(\" -r Reported-by: Sedat Dilek <sedat.dilek@gmail.com> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- arch/arm64/include/asm/cache.h | 2 +- arch/arm64/kernel/smp_spin_table.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.23.0.rc1.153.gdeed80330f-goog