[PATCH v2 0/2] Compiler Attributes: (naked only, for v4.19)

[PATCH v2 0/2] Compiler Attributes: (naked only, for v4.19)

[Miguel Ojeda]

These two patches are the 5th and 6th of the Compiler Attributes series,
which Stefan and Nick requested to go into v4.19 so that the clang ARM32
build is fixed. The v5 of Compiler Attributes will have these two removed
if this goes in.

Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Eli Friedman <efriedma@codeaurora.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joe Perches <joe@perches.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-sparse@vger.kernel.org
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>

Miguel Ojeda (2):
  Compiler Attributes: naked was fixed in gcc 4.6
  Compiler Attributes: naked can be shared

 include/linux/compiler-gcc.h   | 14 --------------
 include/linux/compiler_types.h |  8 ++++++++
 2 files changed, 8 insertions(+), 14 deletions(-)

-- 
2.17.1

[PATCH v2 1/2] Compiler Attributes: naked was fixed in gcc 4.6

[Miguel Ojeda]

Commit 9c695203a7dd ("compiler-gcc.h: gcc-4.5 needs noclone
and noinline on __naked functions") added noinline and noclone
as a workaround for a gcc 4.5 bug, which was resolved in 4.6.0.

Since now the minimum gcc supported version is 4.6,
we can clean it up.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44290
and https://godbolt.org/z/h6NMIL

Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Eli Friedman <efriedma@codeaurora.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joe Perches <joe@perches.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-sparse@vger.kernel.org
Tested-by: Stefan Agner <stefan@agner.ch>
Reviewed-by: Stefan Agner <stefan@agner.ch>
Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
---
 include/linux/compiler-gcc.h | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 763bbad1e258..25d3dd6b2702 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -84,14 +84,8 @@
  * to trace naked functions because then mcount is called without
  * stack and frame pointer being set up and there is no chance to
  * restore the lr register to the value before mcount was called.
- *
- * The asm() bodies of naked functions often depend on standard calling
- * conventions, therefore they must be noinline and noclone.
- *
- * GCC 4.[56] currently fail to enforce this, so we must do so ourselves.
- * See GCC PR44290.
  */
-#define __naked		__attribute__((naked)) noinline __noclone notrace
+#define __naked		__attribute__((naked)) notrace
 
 #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
 
-- 
2.17.1

[PATCH v2 2/2] Compiler Attributes: naked can be shared

[Miguel Ojeda]

The naked attribute is supported by at least gcc >= 4.6 (for ARM,
which is the only current user), gcc >= 8 (for x86), clang >= 3.1
and icc >= 13. See https://godbolt.org/z/350Dyc

Therefore, move it out of compiler-gcc.h so that the definition
is shared by all compilers.

This also fixes Clang support for ARM32 --- 815f0ddb346c
("include/linux/compiler*.h: make compiler-*.h mutually exclusive").

Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Eli Friedman <efriedma@codeaurora.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joe Perches <joe@perches.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-sparse@vger.kernel.org
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Tested-by: Stefan Agner <stefan@agner.ch>
Reviewed-by: Stefan Agner <stefan@agner.ch>
Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
---
 include/linux/compiler-gcc.h   | 8 --------
 include/linux/compiler_types.h | 8 ++++++++
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 25d3dd6b2702..4d36b27214fd 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -79,14 +79,6 @@
 #define __noretpoline __attribute__((indirect_branch("keep")))
 #endif
 
-/*
- * it doesn't make sense on ARM (currently the only user of __naked)
- * to trace naked functions because then mcount is called without
- * stack and frame pointer being set up and there is no chance to
- * restore the lr register to the value before mcount was called.
- */
-#define __naked		__attribute__((naked)) notrace
-
 #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
 
 #define __optimize(level)	__attribute__((__optimize__(level)))
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 3525c179698c..db192becfec4 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -226,6 +226,14 @@ struct ftrace_likely_data {
 #define notrace			__attribute__((no_instrument_function))
 #endif
 
+/*
+ * it doesn't make sense on ARM (currently the only user of __naked)
+ * to trace naked functions because then mcount is called without
+ * stack and frame pointer being set up and there is no chance to
+ * restore the lr register to the value before mcount was called.
+ */
+#define __naked			__attribute__((naked)) notrace
+
 #define __compiler_offsetof(a, b)	__builtin_offsetof(a, b)
 
 /*
-- 
2.17.1

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Greg Kroah-Hartman]

On Tue, Sep 18, 2018 at 06:55:42PM +0200, Miguel Ojeda wrote:
> The naked attribute is supported by at least gcc >= 4.6 (for ARM,
> which is the only current user), gcc >= 8 (for x86), clang >= 3.1
> and icc >= 13. See https://godbolt.org/z/350Dyc
> 
> Therefore, move it out of compiler-gcc.h so that the definition
> is shared by all compilers.
> 
> This also fixes Clang support for ARM32 --- 815f0ddb346c
> ("include/linux/compiler*.h: make compiler-*.h mutually exclusive").

So, with this applied, does clang really build an arm32 kernel
successfully?  No other problem at all?  And this isn't really a
regression, arm32 never really worked with clang yet, right?

thanks,

greg k-h

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Miguel Ojeda]

Hi Greg,

On Tue, Sep 18, 2018 at 7:34 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Sep 18, 2018 at 06:55:42PM +0200, Miguel Ojeda wrote:
>> The naked attribute is supported by at least gcc >= 4.6 (for ARM,
>> which is the only current user), gcc >= 8 (for x86), clang >= 3.1
>> and icc >= 13. See https://godbolt.org/z/350Dyc
>>
>> Therefore, move it out of compiler-gcc.h so that the definition
>> is shared by all compilers.
>>
>> This also fixes Clang support for ARM32 --- 815f0ddb346c
>> ("include/linux/compiler*.h: make compiler-*.h mutually exclusive").
>
> So, with this applied, does clang really build an arm32 kernel
> successfully?  No other problem at all?  And this isn't really a
> regression, arm32 never really worked with clang yet, right?
>

To recap a bit: these two patches come from the "Compiler Attributes"
series which is meant as a general improvement. Since Linus/Andrew/you
didn't comment on whether you wanted or not this for 4.19, we are
assuming they would go in for 4.20. However, Stefan/Nick/... wanted
this for 4.19 instead, they asked me to extract these patches two
separately for 4.19. I let them comment further on the status of Clang
on arm32.

I am going to send a v5 of the entire series without these two
patches, based on -rc4 (or -next, which one do you prefer? I would say
these patches should be applied early in the -next branches, so that
everyone is ready for the change, given it "touches" every translation
unit).

Cheers,
Miguel

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Greg Kroah-Hartman]

On Tue, Sep 18, 2018 at 08:56:04PM +0200, Miguel Ojeda wrote:
> Hi Greg,
> 
> On Tue, Sep 18, 2018 at 7:34 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Tue, Sep 18, 2018 at 06:55:42PM +0200, Miguel Ojeda wrote:
> >> The naked attribute is supported by at least gcc >= 4.6 (for ARM,
> >> which is the only current user), gcc >= 8 (for x86), clang >= 3.1
> >> and icc >= 13. See https://godbolt.org/z/350Dyc
> >>
> >> Therefore, move it out of compiler-gcc.h so that the definition
> >> is shared by all compilers.
> >>
> >> This also fixes Clang support for ARM32 --- 815f0ddb346c
> >> ("include/linux/compiler*.h: make compiler-*.h mutually exclusive").
> >
> > So, with this applied, does clang really build an arm32 kernel
> > successfully?  No other problem at all?  And this isn't really a
> > regression, arm32 never really worked with clang yet, right?
> >
> 
> To recap a bit: these two patches come from the "Compiler Attributes"
> series which is meant as a general improvement.

Ok, so that's not for regressions, that's fine.

> Since Linus/Andrew/you
> didn't comment on whether you wanted or not this for 4.19, we are
> assuming they would go in for 4.20. However, Stefan/Nick/... wanted
> this for 4.19 instead, they asked me to extract these patches two
> separately for 4.19. I let them comment further on the status of Clang
> on arm32.

If these do not fix a regression, I don't see how they would be ready
for 4.19-final.

> I am going to send a v5 of the entire series without these two
> patches, based on -rc4 (or -next, which one do you prefer? I would say
> these patches should be applied early in the -next branches, so that
> everyone is ready for the change, given it "touches" every translation
> unit).

That's up to whomever takes these into their tree for linux-next
inclusion.  If you are about to break everything, then you might
consider changing your patches so they do not do that :)

good luck!

greg k-h

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Miguel Ojeda]

On Wed, Sep 19, 2018 at 11:14 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Sep 18, 2018 at 08:56:04PM +0200, Miguel Ojeda wrote:
>> Hi Greg,
>>
>
>> Since Linus/Andrew/you
>> didn't comment on whether you wanted or not this for 4.19, we are
>> assuming they would go in for 4.20. However, Stefan/Nick/... wanted
>> this for 4.19 instead, they asked me to extract these patches two
>> separately for 4.19. I let them comment further on the status of Clang
>> on arm32.
>
> If these do not fix a regression, I don't see how they would be ready
> for 4.19-final.

Ok, I will wait a bit to send v5 until this is sorted out.

[CC'd Nick, Stefan, Arnd: I just noticed the Reviewed-by/... lines
were not picked as CC].

>
>> I am going to send a v5 of the entire series without these two
>> patches, based on -rc4 (or -next, which one do you prefer? I would say
>> these patches should be applied early in the -next branches, so that
>> everyone is ready for the change, given it "touches" every translation
>> unit).
>
> That's up to whomever takes these into their tree for linux-next
> inclusion.  If you are about to break everything, then you might
> consider changing your patches so they do not do that :)
>

Well, the series shouldn't break anything (famous last words :), even
if everyone includes those headers. So, in theory, they *could* be
applied anywhere, anytime; but given they are global changes...

Cheers,
Miguel

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Stefan Agner]

On 19.09.2018 16:00, Miguel Ojeda wrote:
> On Wed, Sep 19, 2018 at 11:14 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Tue, Sep 18, 2018 at 08:56:04PM +0200, Miguel Ojeda wrote:
>>> Hi Greg,
>>>
>>
>>> Since Linus/Andrew/you
>>> didn't comment on whether you wanted or not this for 4.19, we are
>>> assuming they would go in for 4.20. However, Stefan/Nick/... wanted
>>> this for 4.19 instead, they asked me to extract these patches two
>>> separately for 4.19. I let them comment further on the status of Clang
>>> on arm32.
>>
>> If these do not fix a regression, I don't see how they would be ready
>> for 4.19-final.

Clang on arm32 worked with v4.18 when using multi_v7_defconfig -CONFIG_EFI.

With 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
exclusive") it broke on v4.19-rc1. IMHO this is a regression and we should
consider this two patches as a fix for it.


> 
> Ok, I will wait a bit to send v5 until this is sorted out.
> 
> [CC'd Nick, Stefan, Arnd: I just noticed the Reviewed-by/... lines
> were not picked as CC].

Oh yeah thanks, really did not notice the discussion around v2 until
you CC'd me now.

--
Stefan

> 
>>
>>> I am going to send a v5 of the entire series without these two
>>> patches, based on -rc4 (or -next, which one do you prefer? I would say
>>> these patches should be applied early in the -next branches, so that
>>> everyone is ready for the change, given it "touches" every translation
>>> unit).
>>
>> That's up to whomever takes these into their tree for linux-next
>> inclusion.  If you are about to break everything, then you might
>> consider changing your patches so they do not do that :)
>>
> 
> Well, the series shouldn't break anything (famous last words :), even
> if everyone includes those headers. So, in theory, they *could* be
> applied anywhere, anytime; but given they are global changes...
> 
> Cheers,
> Miguel

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Greg Kroah-Hartman]

On Wed, Sep 19, 2018 at 11:00:37PM -0700, Stefan Agner wrote:
> On 19.09.2018 16:00, Miguel Ojeda wrote:
> > On Wed, Sep 19, 2018 at 11:14 PM, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> >> On Tue, Sep 18, 2018 at 08:56:04PM +0200, Miguel Ojeda wrote:
> >>> Hi Greg,
> >>>
> >>
> >>> Since Linus/Andrew/you
> >>> didn't comment on whether you wanted or not this for 4.19, we are
> >>> assuming they would go in for 4.20. However, Stefan/Nick/... wanted
> >>> this for 4.19 instead, they asked me to extract these patches two
> >>> separately for 4.19. I let them comment further on the status of Clang
> >>> on arm32.
> >>
> >> If these do not fix a regression, I don't see how they would be ready
> >> for 4.19-final.
> 
> Clang on arm32 worked with v4.18 when using multi_v7_defconfig -CONFIG_EFI.

That's a narrow definition :)

> With 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
> exclusive") it broke on v4.19-rc1. IMHO this is a regression and we should
> consider this two patches as a fix for it.

Why not just revert 815f0ddb346c instead and work to get it right for
4.20-rc1?

thanks,

greg k-h

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Greg Kroah-Hartman]

On Thu, Sep 20, 2018 at 01:00:41AM +0200, Miguel Ojeda wrote:
> >> I am going to send a v5 of the entire series without these two
> >> patches, based on -rc4 (or -next, which one do you prefer? I would say
> >> these patches should be applied early in the -next branches, so that
> >> everyone is ready for the change, given it "touches" every translation
> >> unit).
> >
> > That's up to whomever takes these into their tree for linux-next
> > inclusion.  If you are about to break everything, then you might
> > consider changing your patches so they do not do that :)
> >
> 
> Well, the series shouldn't break anything (famous last words :), even
> if everyone includes those headers. So, in theory, they *could* be
> applied anywhere, anytime; but given they are global changes...

It doesn't matter the "order" in which global changes are added to
linux-next.  If you think it does, please work with the linux-next
maintainer to properly place your tree in the correct "location".

thanks,

greg k-h

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Dominique Martinet]

Greg Kroah-Hartman wrote on Wed, Sep 19, 2018:
> > > So, with this applied, does clang really build an arm32 kernel
> > > successfully?  No other problem at all?  And this isn't really a
> > > regression, arm32 never really worked with clang yet, right?
> > 
> > To recap a bit: these two patches come from the "Compiler Attributes"
> > series which is meant as a general improvement.
> 
> Ok, so that's not for regressions, that's fine.

I've not followed so closely, in particular I'm not sure if it's the
only problem with arm32 right now, but that is a regression - the
general serie is meant as an improvement, but these two patches fix
815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
exclusive") which was taken in 4.19-rc1

(Miguel, perhaps a Fixes: tag might help remember that?)

> If these do not fix a regression, I don't see how they would be ready
> for 4.19-final.

So the rest of the series is not a regression and isn't ready for
4.19-final, these two would be, but only got tested by the handful of
people in Cc here ; so ultimately it's your call.


> > I am going to send a v5 of the entire series without these two
> > patches, based on -rc4 (or -next, which one do you prefer? I would say
> > these patches should be applied early in the -next branches, so that
> > everyone is ready for the change, given it "touches" every translation
> > unit).
> 
> That's up to whomever takes these into their tree for linux-next
> inclusion.  If you are about to break everything, then you might
> consider changing your patches so they do not do that :)

I think that was more or less his question, there is no maintainer for
these files, so who should that whomever be? :)
The thing is linux took this kind of patch directly last time, but
ideally it really should sink in -next for a bit...

(If no-one in Cc has anything included in -next I could take them in my
9p branch, but that is quite a bit out of scope from what I advertised
this branch as so I think it would be confusing ; I think it might
almost be best if Miguel will keep maintaining these in the future to do
his own request to inclusion in -next?)

-- 
Dominique Martinet

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Miguel Ojeda]

Hi Dominique,

On Thu, Sep 20, 2018 at 1:05 AM, Dominique Martinet
<asmadeus@codewreck.org> wrote:
> Greg Kroah-Hartman wrote on Wed, Sep 19, 2018:
>> > > So, with this applied, does clang really build an arm32 kernel
>> > > successfully?  No other problem at all?  And this isn't really a
>> > > regression, arm32 never really worked with clang yet, right?
>> >
>> > To recap a bit: these two patches come from the "Compiler Attributes"
>> > series which is meant as a general improvement.
>>
>> Ok, so that's not for regressions, that's fine.
>
> I've not followed so closely, in particular I'm not sure if it's the
> only problem with arm32 right now, but that is a regression - the
> general serie is meant as an improvement, but these two patches fix
> 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
> exclusive") which was taken in 4.19-rc1
>
> (Miguel, perhaps a Fixes: tag might help remember that?)

The commit is mentioned in the commit message, although not with the
Fixes: syntax -- is something now automatically parsing that? I guess
it is also easier for humans to parse :)

>
>> If these do not fix a regression, I don't see how they would be ready
>> for 4.19-final.
>
> So the rest of the series is not a regression and isn't ready for
> 4.19-final, these two would be, but only got tested by the handful of
> people in Cc here ; so ultimately it's your call.
>

To further clarify about the series: it is probably fine for 4.19 with
the latest tests/reviews we did (v4), but it is too late for such a
change in the cycle (and due to this I am taking the chance to merge
the nonstring series into the Compiler Attributes one).

>
>> > I am going to send a v5 of the entire series without these two
>> > patches, based on -rc4 (or -next, which one do you prefer? I would say
>> > these patches should be applied early in the -next branches, so that
>> > everyone is ready for the change, given it "touches" every translation
>> > unit).
>>
>> That's up to whomever takes these into their tree for linux-next
>> inclusion.  If you are about to break everything, then you might
>> consider changing your patches so they do not do that :)
>
> I think that was more or less his question, there is no maintainer for
> these files, so who should that whomever be? :)
> The thing is linux took this kind of patch directly last time, but
> ideally it really should sink in -next for a bit...

Yeah, Linus is (was...) aware of it, so initially I thought Linus
would pick this whenever he felt it was ready.

>
> (If no-one in Cc has anything included in -next I could take them in my
> 9p branch, but that is quite a bit out of scope from what I advertised
> this branch as so I think it would be confusing ; I think it might
> almost be best if Miguel will keep maintaining these in the future to do
> his own request to inclusion in -next?)

I can ask for my auxdisplay tree (or better, a new one) to be in -next
and use that (are non-kernel.org trees allowed to be in -next, by the
way?).

Cheers,
Miguel

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Dominique Martinet]

Miguel Ojeda wrote on Thu, Sep 20, 2018:
> > I've not followed so closely, in particular I'm not sure if it's the
> > only problem with arm32 right now, but that is a regression - the
> > general serie is meant as an improvement, but these two patches fix
> > 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
> > exclusive") which was taken in 4.19-rc1
> >
> > (Miguel, perhaps a Fixes: tag might help remember that?)
> 
> The commit is mentioned in the commit message, although not with the
> Fixes: syntax -- is something now automatically parsing that? I guess
> it is also easier for humans to parse :)

As far as I'm aware, the backport to stable stuff parse these to know up
till how far back they should backport, but it still requires explicit
Cc to the stable@vger list... (not needed here as the "bad" commit never
made it to stable)
I'm not aware of anything else, but as you said, while I now see you
naming the commit now, I managed to miss it earlier and I was somewhat
following this so it's probably also easier on humans :)

> > (If no-one in Cc has anything included in -next I could take them in my
> > 9p branch, but that is quite a bit out of scope from what I advertised
> > this branch as so I think it would be confusing ; I think it might
> > almost be best if Miguel will keep maintaining these in the future to do
> > his own request to inclusion in -next?)
> 
> I can ask for my auxdisplay tree (or better, a new one) to be in -next
> and use that (are non-kernel.org trees allowed to be in -next, by the
> way?).

I think a new one would be great, yes.
(my branch is on github, so Stephen does not appear to mind
non-kernel.org trees)

-- 
Dominique Matinet

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Greg Kroah-Hartman]

On Thu, Sep 20, 2018 at 02:10:24AM +0200, Dominique Martinet wrote:
> Miguel Ojeda wrote on Thu, Sep 20, 2018:
> > > I've not followed so closely, in particular I'm not sure if it's the
> > > only problem with arm32 right now, but that is a regression - the
> > > general serie is meant as an improvement, but these two patches fix
> > > 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
> > > exclusive") which was taken in 4.19-rc1
> > >
> > > (Miguel, perhaps a Fixes: tag might help remember that?)
> > 
> > The commit is mentioned in the commit message, although not with the
> > Fixes: syntax -- is something now automatically parsing that? I guess
> > it is also easier for humans to parse :)
> 
> As far as I'm aware, the backport to stable stuff parse these to know up
> till how far back they should backport, but it still requires explicit
> Cc to the stable@vger list... (not needed here as the "bad" commit never
> made it to stable)
> I'm not aware of anything else, but as you said, while I now see you
> naming the commit now, I managed to miss it earlier and I was somewhat
> following this so it's probably also easier on humans :)

"Fixes:" is not just for stable, we use it wherever we have a patch that
we know fixes a problem introduced in another patch.

For this instance, I think we should just revert the offending patch,
which should resolve the issue for everyone and then you can try to redo
your series to get it right the next time.

Sound good?

> > > (If no-one in Cc has anything included in -next I could take them in my
> > > 9p branch, but that is quite a bit out of scope from what I advertised
> > > this branch as so I think it would be confusing ; I think it might
> > > almost be best if Miguel will keep maintaining these in the future to do
> > > his own request to inclusion in -next?)
> > 
> > I can ask for my auxdisplay tree (or better, a new one) to be in -next
> > and use that (are non-kernel.org trees allowed to be in -next, by the
> > way?).
> 
> I think a new one would be great, yes.
> (my branch is on github, so Stephen does not appear to mind
> non-kernel.org trees)

Why not just route these through Andrew?  He takes lots of stuff like
this for this very reason.

thanks,

greg k-h

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Dominique Martinet]

Greg Kroah-Hartman wrote on Thu, Sep 20, 2018:
> "Fixes:" is not just for stable, we use it wherever we have a patch that
> we know fixes a problem introduced in another patch.
> 
> For this instance, I think we should just revert the offending patch,
> which should resolve the issue for everyone and then you can try to redo
> your series to get it right the next time.
> 
> Sound good?

Except that 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h
mutually exclusive") itself fixes cafa0010cd51 ("Raise the minimum
required gcc version to 4.6"), which breaks clang altogether (as used by
example by bcc for most BPF programs, that I caught before -rc1 got
released so we got both in rc1)

I'm not aware of anything that would break if both were to be reverted,
I have no opinion on which way to go.

> Why not just route these through Andrew?  He takes lots of stuff like
> this for this very reason.

That works for me (although it might have helped if Andrew had been in
Cc at any point in this discussion...), but part of the discussion was
about seriously maintaining these files, and Miguel stepped up to help
with that so it could make sense to have his own tree.


Frankly, after this whole episode I'd find quite helpful if "compiler
stuff" (or headers maintainance in general) were to grow its own mailing
list and start being considered like a proper component of the kernel.

It does impact quite a few people, and it's neigh-impossible to review
this stuff as things are right now with a hand-picked list of CCs, no
matter how large it is -- I don't mind if it goes in -next through its
own branch or through Andrew, but a proper place where folks interested
in these could subscribe and test/review the patches would be awesome.

-- 
Dominique Martinet

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Geert Uytterhoeven]

On Thu, Sep 20, 2018 at 9:37 AM Dominique Martinet
<asmadeus@codewreck.org> wrote:
> Greg Kroah-Hartman wrote on Thu, Sep 20, 2018:
> > "Fixes:" is not just for stable, we use it wherever we have a patch that
> > we know fixes a problem introduced in another patch.
> >
> > For this instance, I think we should just revert the offending patch,
> > which should resolve the issue for everyone and then you can try to redo
> > your series to get it right the next time.
> >
> > Sound good?
>
> Except that 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h
> mutually exclusive") itself fixes cafa0010cd51 ("Raise the minimum
> required gcc version to 4.6"), which breaks clang altogether (as used by
> example by bcc for most BPF programs, that I caught before -rc1 got
> released so we got both in rc1)
>
> I'm not aware of anything that would break if both were to be reverted,
> I have no opinion on which way to go.

I guess reverting them makes no difference for gcc >= 4.6.

For older compilers (which were declared unsupported by cafa0010cd51),
you also need
https://lore.kernel.org/lkml/20180814160208.4f4dd7ca142912f5894ddddd@linux-foundation.org/

Been there, done that, happy gcc-4.1.2 ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Miguel Ojeda]

On Thu, Sep 20, 2018 at 9:36 AM, Dominique Martinet
<asmadeus@codewreck.org> wrote:
> Greg Kroah-Hartman wrote on Thu, Sep 20, 2018:
>> Why not just route these through Andrew?  He takes lots of stuff like
>> this for this very reason.
>
> That works for me (although it might have helped if Andrew had been in
> Cc at any point in this discussion...), but part of the discussion was

Cc'ing him (and Luc too) now. I have put a bigger list of Cc people
for v5 (and will simply Cc everyone for all patches).

> about seriously maintaining these files, and Miguel stepped up to help
> with that so it could make sense to have his own tree.
>
>
> Frankly, after this whole episode I'd find quite helpful if "compiler
> stuff" (or headers maintainance in general) were to grow its own mailing
> list and start being considered like a proper component of the kernel.

Yeah, the compiler headers have seen quite some changes in the last ~3
years compared to the time before so it might be a good idea:

  https://ojeda.io/uploads/compiler-headers-file-size.png

>> It does impact quite a few people, and it's neigh-impossible to review
> this stuff as things are right now with a hand-picked list of CCs, no
> matter how large it is -- I don't mind if it goes in -next through its
> own branch or through Andrew, but a proper place where folks interested
> in these could subscribe and test/review the patches would be awesome.
>

I guess compiler folks would like that too.

Cheers,
Miguel

Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared

[Miguel Ojeda]

On Thu, Sep 20, 2018 at 9:22 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> For this instance, I think we should just revert the offending patch,
> which should resolve the issue for everyone and then you can try to redo
> your series to get it right the next time.
>
> Sound good?

On one hand, "reverting & retrying" is a good default policy. On the
other hand, we are already in -rc4 (i.e. we lose the testing done
until now --- note that in this case the revert implies a global
change). So whatever makes you guys feel more comfortable.

Cheers,
Miguel

Re: [PATCH v2 0/2] Compiler Attributes: (naked only, for v4.19)

[Greg Kroah-Hartman]

On Tue, Sep 18, 2018 at 06:55:40PM +0200, Miguel Ojeda wrote:
> These two patches are the 5th and 6th of the Compiler Attributes series,
> which Stefan and Nick requested to go into v4.19 so that the clang ARM32
> build is fixed. The v5 of Compiler Attributes will have these two removed
> if this goes in.

As these seem pretty "obvious" and they do fix a problem for one
system/compiler that we want to support, and because odds are I would
end up getting them submitted to the 4.19-stable tree eventually, I've
now applied them to the tree.

thanks,

greg k-h

Re: [PATCH v2 0/2] Compiler Attributes: (naked only, for v4.19)

[Greg Kroah-Hartman]

On Thu, Sep 20, 2018 at 03:57:48PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Sep 18, 2018 at 06:55:40PM +0200, Miguel Ojeda wrote:
> > These two patches are the 5th and 6th of the Compiler Attributes series,
> > which Stefan and Nick requested to go into v4.19 so that the clang ARM32
> > build is fixed. The v5 of Compiler Attributes will have these two removed
> > if this goes in.
> 
> As these seem pretty "obvious" and they do fix a problem for one
> system/compiler that we want to support, and because odds are I would
> end up getting them submitted to the 4.19-stable tree eventually, I've
> now applied them to the tree.

Also, it passed my build tests, I didn't just throw these in without
doing that :)

greg k-h

Re: [PATCH v2 0/2] Compiler Attributes: (naked only, for v4.19)

[Miguel Ojeda]

On Thu, Sep 20, 2018 at 3:59 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Sep 20, 2018 at 03:57:48PM +0200, Greg Kroah-Hartman wrote:
>> On Tue, Sep 18, 2018 at 06:55:40PM +0200, Miguel Ojeda wrote:
>> > These two patches are the 5th and 6th of the Compiler Attributes series,
>> > which Stefan and Nick requested to go into v4.19 so that the clang ARM32
>> > build is fixed. The v5 of Compiler Attributes will have these two removed
>> > if this goes in.
>>
>> As these seem pretty "obvious" and they do fix a problem for one
>> system/compiler that we want to support, and because odds are I would
>> end up getting them submitted to the 4.19-stable tree eventually, I've
>> now applied them to the tree.

Seems the easiest way forward for everyone, indeed.

>
> Also, it passed my build tests, I didn't just throw these in without
> doing that :)

Thanks for double-checking! Sending in a bit the v5 of Compiler
Attributes rebased on this commit then.

Cheers,
Miguel