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 <[email protected]>
Cc: Eli Friedman <[email protected]>
Cc: Christopher Li <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Dominique Martinet <[email protected]>
Cc: Stefan Agner <[email protected]>
Cc: Luc Van Oostenryck <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
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
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 <[email protected]>
Cc: Eli Friedman <[email protected]>
Cc: Christopher Li <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Dominique Martinet <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Tested-by: Stefan Agner <[email protected]>
Reviewed-by: Stefan Agner <[email protected]>
Reviewed-by: Luc Van Oostenryck <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
---
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
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 <[email protected]>
Cc: Eli Friedman <[email protected]>
Cc: Christopher Li <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Dominique Martinet <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Suggested-by: Arnd Bergmann <[email protected]>
Tested-by: Stefan Agner <[email protected]>
Reviewed-by: Stefan Agner <[email protected]>
Reviewed-by: Luc Van Oostenryck <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
---
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
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
Hi Greg,
On Tue, Sep 18, 2018 at 7:34 PM, Greg Kroah-Hartman
<[email protected]> 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
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
> <[email protected]> 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
On Wed, Sep 19, 2018 at 11:14 PM, Greg Kroah-Hartman
<[email protected]> 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
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
Hi Dominique,
On Thu, Sep 20, 2018 at 1:05 AM, Dominique Martinet
<[email protected]> 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
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
On 19.09.2018 16:00, Miguel Ojeda wrote:
> On Wed, Sep 19, 2018 at 11:14 PM, Greg Kroah-Hartman
> <[email protected]> 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
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
> > <[email protected]> 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
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
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
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
On Thu, Sep 20, 2018 at 9:37 AM Dominique Martinet
<[email protected]> 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/[email protected]/
Been there, done that, happy gcc-4.1.2 ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Thu, Sep 20, 2018 at 9:22 AM, Greg Kroah-Hartman
<[email protected]> 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
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
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
On Thu, Sep 20, 2018 at 9:36 AM, Dominique Martinet
<[email protected]> 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
On Thu, Sep 20, 2018 at 3:59 PM, Greg Kroah-Hartman
<[email protected]> 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
