Re: Bad MAINTAINERS pattern in section 'PARALLEL LCD/KEYPAD PANEL DRIVER'
Hi Joe, On Sat, Sep 29, 2018 at 12:03 AM Joe Perches wrote: > > Please fix this defect appropriately. Thanks, will send a PR. Cheers, Miguel
Re: [PATCH] drivers/auxdisplay: TRIVIAl removed unnessecary braces from if statements
Hi Ben,
On Sun, Sep 23, 2018 at 4:33 PM, wrote:
> From: Ben Moore
>
> removed braches from single line if statements
>
> Signed-off-by: Ben Moore
> ---
> drivers/auxdisplay/panel.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
> index 3b25a643058c..0e4a7260dcc0 100644
> --- a/drivers/auxdisplay/panel.c
> +++ b/drivers/auxdisplay/panel.c
> @@ -695,11 +695,10 @@ static void lcd_send_serial(int byte)
> for (bit = 0; bit < 8; bit++) {
> clear_bit(LCD_BIT_CL, bits);/* CLK low */
> panel_set_bits();
> - if (byte & 1) {
> + if (byte & 1)
> set_bit(LCD_BIT_DA, bits);
> - } else {
> + else
> clear_bit(LCD_BIT_DA, bits);
> - }
>
> panel_set_bits();
> udelay(2); /* maintain the data during 2 us before CLK up */
> @@ -1025,9 +1024,8 @@ static void lcd_init(void)
> lcd.pins.rs = DEFAULT_LCD_PIN_RS;
> if (lcd.pins.rw == PIN_NOT_SET)
> lcd.pins.rw = DEFAULT_LCD_PIN_RW;
> - } else {
> + } else
> charlcd->ops = _tilcd_ops;
> - }
>
> if (lcd.pins.bl == PIN_NOT_SET)
> lcd.pins.bl = DEFAULT_LCD_PIN_BL;
> --
> 2.11.0
>
A few more things (Ben's first patch, sent to me first in private):
* The subject could be better as:
[PATCH TRIVIAL] auxdisplay: remove unnecessary braces
i.e. TRIVIAL inside the [...] (which will not appear in the Git
commit message); unnessecary -> unnecessary; drivers/auxdisplay ->
auxdisplay
* The commit message body ("removed braches from single line if
statements") is redundant with the title of the commit (and has the
typo "braches"). You could say, instead, say something like "No
functional changes."; or if you tested that it generates the same
object file, something like "Tested, no codegen changes."; etc.
* You can remove the "From:" line.
* I think Willy uses the w...@1wt.eu address nowadays.
* It would be great if you can try to look for similar instances in
drivers/auxdisplay/* and send a v2 of the patch (you would need to say
in the subject "[PATCH v2 TRIVIAL]").
* Don't forget to Cc the general linux-kernel mailing list, please!
Thank again for the patch and welcome to the kernel!
[Jiri, I can pick the v2 of this in my tree if you prefer.]
Cheers,
Miguel
Re: [PATCH] drivers/auxdisplay: TRIVIAl removed unnessecary braces from if statements
Hi Ben,
On Sun, Sep 23, 2018 at 4:33 PM, wrote:
> From: Ben Moore
>
> removed braches from single line if statements
>
> Signed-off-by: Ben Moore
> ---
> drivers/auxdisplay/panel.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
> index 3b25a643058c..0e4a7260dcc0 100644
> --- a/drivers/auxdisplay/panel.c
> +++ b/drivers/auxdisplay/panel.c
> @@ -695,11 +695,10 @@ static void lcd_send_serial(int byte)
> for (bit = 0; bit < 8; bit++) {
> clear_bit(LCD_BIT_CL, bits);/* CLK low */
> panel_set_bits();
> - if (byte & 1) {
> + if (byte & 1)
> set_bit(LCD_BIT_DA, bits);
> - } else {
> + else
> clear_bit(LCD_BIT_DA, bits);
> - }
>
> panel_set_bits();
> udelay(2); /* maintain the data during 2 us before CLK up */
> @@ -1025,9 +1024,8 @@ static void lcd_init(void)
> lcd.pins.rs = DEFAULT_LCD_PIN_RS;
> if (lcd.pins.rw == PIN_NOT_SET)
> lcd.pins.rw = DEFAULT_LCD_PIN_RW;
> - } else {
> + } else
> charlcd->ops = _tilcd_ops;
> - }
>
> if (lcd.pins.bl == PIN_NOT_SET)
> lcd.pins.bl = DEFAULT_LCD_PIN_BL;
> --
> 2.11.0
>
A few more things (Ben's first patch, sent to me first in private):
* The subject could be better as:
[PATCH TRIVIAL] auxdisplay: remove unnecessary braces
i.e. TRIVIAL inside the [...] (which will not appear in the Git
commit message); unnessecary -> unnecessary; drivers/auxdisplay ->
auxdisplay
* The commit message body ("removed braches from single line if
statements") is redundant with the title of the commit (and has the
typo "braches"). You could say, instead, say something like "No
functional changes."; or if you tested that it generates the same
object file, something like "Tested, no codegen changes."; etc.
* You can remove the "From:" line.
* I think Willy uses the w...@1wt.eu address nowadays.
* It would be great if you can try to look for similar instances in
drivers/auxdisplay/* and send a v2 of the patch (you would need to say
in the subject "[PATCH v2 TRIVIAL]").
* Don't forget to Cc the general linux-kernel mailing list, please!
Thank again for the patch and welcome to the kernel!
[Jiri, I can pick the v2 of this in my tree if you prefer.]
Cheers,
Miguel
Re: [PATCH TRIVIAL] Punctuation fixes
Hi Richard, On Sun, Sep 23, 2018 at 10:59 AM, Richard Weinberger wrote: > On Sun, Sep 23, 2018 at 3:00 AM Miguel Ojeda > wrote: >> >> Hi Diego, >> >> A few things, since it looks like this is your first patch. > > git log... Oops! I guess I mistyped the search string, sorry Diego! Cheers, Miguel
Re: [PATCH TRIVIAL] Punctuation fixes
Hi Richard, On Sun, Sep 23, 2018 at 10:59 AM, Richard Weinberger wrote: > On Sun, Sep 23, 2018 at 3:00 AM Miguel Ojeda > wrote: >> >> Hi Diego, >> >> A few things, since it looks like this is your first patch. > > git log... Oops! I guess I mistyped the search string, sorry Diego! Cheers, Miguel
Re: [PATCH TRIVIAL] Punctuation fixes
Hi Diego, A few things, since it looks like this is your first patch. On Sat, Sep 22, 2018 at 5:56 PM, Diego Viola wrote: > On Wed, Sep 12, 2018 at 12:54 AM Diego Viola wrote: >> >> Signed-off-by: Diego Viola >> --- >> CREDITS | 2 +- >> MAINTAINERS | 2 +- >> Makefile| 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/CREDITS b/CREDITS >> index 5befd2d71..b82efb36d 100644 >> --- a/CREDITS >> +++ b/CREDITS >> @@ -1473,7 +1473,7 @@ W: http://www.linux-ide.org/ >> W: http://www.linuxdiskcert.org/ >> D: Random SMP kernel hacker... >> D: Uniform Multi-Platform E-IDE driver >> -D: Active-ATA-Chipset maddness.. >> +D: Active-ATA-Chipset maddness... I think the extra dots is what was intended here :) >> D: Ultra DMA 133/100/66/33 w/48-bit Addressing >> D: ATA-Disconnect, ATA-TCQ >> D: ATA-Smart Kernel Daemon >> diff --git a/MAINTAINERS b/MAINTAINERS >> index d870cb57c..6567bf245 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -93,7 +93,7 @@ Descriptions of section entries: >>Supported: Someone is actually paid to look after this. >>Maintained: Someone actually looks after it. >>Odd Fixes: It has a maintainer but they don't have time to do >> - much other than throw the odd patch in. See below.. >> + much other than throw the odd patch in. See below. >>Orphan: No current maintainer [but maybe you could take the >> role as you write your new code]. >>Obsolete:Old code. Something tagged obsolete generally means >> diff --git a/Makefile b/Makefile >> index 4d5c883a9..7b5c5d634 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -1109,7 +1109,7 @@ archprepare: archheaders archscripts prepare1 >> scripts_basic >> prepare0: archprepare gcc-plugins >> $(Q)$(MAKE) $(build)=. >> >> -# All the preparing.. >> +# All the preparing... >> prepare: prepare0 prepare-objtool >> >> # Support for using generic headers in asm-generic >> -- >> 2.19.0 >> > > Jonathan, > > can you please merge my patch? After you merge this one, I'll go > through the rest of the documentation to make it more consistent. Jonathan is not responsible for these files. Jiri Kosina is the maintainer of the trivial tree. There are 1700+ instances of a "two-stops ellipsis" in the kernel. I am not sure they were mistakes, given their prevalence, even if they are not formal/common English. Also, I am not sure a commit for each 3 is really appropriate! :) Maybe do the full sweep of the Documentation ones in a single patch? Also, typically you would note in the subject that this is a "v2" of your previous patch (if I am not wrong, you sent a first version fixing only one of the 3 mistakes). Thanks for your first patch! Cheers, Miguel
Re: [PATCH TRIVIAL] Punctuation fixes
Hi Diego, A few things, since it looks like this is your first patch. On Sat, Sep 22, 2018 at 5:56 PM, Diego Viola wrote: > On Wed, Sep 12, 2018 at 12:54 AM Diego Viola wrote: >> >> Signed-off-by: Diego Viola >> --- >> CREDITS | 2 +- >> MAINTAINERS | 2 +- >> Makefile| 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/CREDITS b/CREDITS >> index 5befd2d71..b82efb36d 100644 >> --- a/CREDITS >> +++ b/CREDITS >> @@ -1473,7 +1473,7 @@ W: http://www.linux-ide.org/ >> W: http://www.linuxdiskcert.org/ >> D: Random SMP kernel hacker... >> D: Uniform Multi-Platform E-IDE driver >> -D: Active-ATA-Chipset maddness.. >> +D: Active-ATA-Chipset maddness... I think the extra dots is what was intended here :) >> D: Ultra DMA 133/100/66/33 w/48-bit Addressing >> D: ATA-Disconnect, ATA-TCQ >> D: ATA-Smart Kernel Daemon >> diff --git a/MAINTAINERS b/MAINTAINERS >> index d870cb57c..6567bf245 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -93,7 +93,7 @@ Descriptions of section entries: >>Supported: Someone is actually paid to look after this. >>Maintained: Someone actually looks after it. >>Odd Fixes: It has a maintainer but they don't have time to do >> - much other than throw the odd patch in. See below.. >> + much other than throw the odd patch in. See below. >>Orphan: No current maintainer [but maybe you could take the >> role as you write your new code]. >>Obsolete:Old code. Something tagged obsolete generally means >> diff --git a/Makefile b/Makefile >> index 4d5c883a9..7b5c5d634 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -1109,7 +1109,7 @@ archprepare: archheaders archscripts prepare1 >> scripts_basic >> prepare0: archprepare gcc-plugins >> $(Q)$(MAKE) $(build)=. >> >> -# All the preparing.. >> +# All the preparing... >> prepare: prepare0 prepare-objtool >> >> # Support for using generic headers in asm-generic >> -- >> 2.19.0 >> > > Jonathan, > > can you please merge my patch? After you merge this one, I'll go > through the rest of the documentation to make it more consistent. Jonathan is not responsible for these files. Jiri Kosina is the maintainer of the trivial tree. There are 1700+ instances of a "two-stops ellipsis" in the kernel. I am not sure they were mistakes, given their prevalence, even if they are not formal/common English. Also, I am not sure a commit for each 3 is really appropriate! :) Maybe do the full sweep of the Documentation ones in a single patch? Also, typically you would note in the subject that this is a "v2" of your previous patch (if I am not wrong, you sent a first version fixing only one of the 3 mistakes). Thanks for your first patch! Cheers, Miguel
Re: [PATCH] auxdisplay/cfag12864bfb.c: Replace vm_insert_page
On Fri, Sep 21, 2018 at 1:07 PM, Souptick Joarder wrote:
> On Fri, Sep 21, 2018 at 2:56 AM Miguel Ojeda
> wrote:
>>
>> A link to the discussion/plan would be nice. The commit 1c8f422059ae5
>> ("mm: change return type to vm_fault_t") explains a bit, but has a
>> broken link :( Googling for the stuff returns many of the patches, but
>> not the actual discussion...
>
> This might be helpful.
> https://marc.info/?l=linux-mm=152054772413234=4
Thank you, although that does not explain why the changes are
happening, only that you are introducing the new API so that you can
start converting users (which is the normal way of doing it, no?).
What I meant is the discussion that led to commit 1c8f422059ae5 itself
(which has a link inside, but is broken).
>> I am out of the loop on these mm changes, so please indulge me, but:
>>
>> * Why is there no documentation on vmf_insert_page() while
>> vm_insert_page() had it? (specially since it seems you want to remove
>> vm_insert_page()).
>
> The plan is to convert vm_insert_{page,pfn,mixed} to
> vmf_insert_{page,pfn,mixed}. As a good intermediate
> steps inline wrapper vmf_insert_{pfn,page,mixed} are
> introduced. After all the drivers converted, we will convert
> vm_insert_page to vmf_insert_page and remove the inline
> wrapper and update the document at the same time.
Yeah, that is what 1c8f422059ae5 ("mm: change return type to
vm_fault_t") seems to say at the end (thanks for clarifying).
Still, that does not explain why the documentation was not added at
the same time as soon the new API is introduced. I don't see how it
matters that they are wrappers.
Actually, I think the wrappers should have been the final functions
already in memory.c, their declarations in mm.h, etc. That way you
would minimize the code changes later on: you would be only removing
dead code, rather than changing code again. Even if you forward the
calls for the moment, it would have been a much smaller change later
on.
>
>>
>> * Shouldn't we have a simple remap_page() or remap_kernel_page() to
>> fit this use case and avoid that dance? (another driver in auxdisplay
>> will require the same change, and I guess others in the kernel as
>> well).
>
>
> There are few drivers similar like auxdisplay where straight forward
> conversion from vm_insert_page to vmf_insert_page is not possible.
>
> So I mapped the kernel memory to user vma using remap_pfn_range
> and remove vm_insert_page in this driver.
>
> Other way, is to replace vm_insert_page with vmf_insert_page() and
> then convert VM_FAULT_CODE back to errno. But as part of vm_fault_t
> migration we have already removed/cleanup most the errno to VM_FAULT_CODE
> mapping from drivers. So I prefer not to take this option.
>
> Third, we can introduce a similar API like vm_insert_page say,
> vm_insert_kmem_page() and use it for same scenarios like this.
Yep, I think that is the best, unless there are only a couple of users
and you think nobody should be using it in the future.
Cheers,
Miguel
Re: [PATCH] auxdisplay/cfag12864bfb.c: Replace vm_insert_page
On Fri, Sep 21, 2018 at 1:07 PM, Souptick Joarder wrote:
> On Fri, Sep 21, 2018 at 2:56 AM Miguel Ojeda
> wrote:
>>
>> A link to the discussion/plan would be nice. The commit 1c8f422059ae5
>> ("mm: change return type to vm_fault_t") explains a bit, but has a
>> broken link :( Googling for the stuff returns many of the patches, but
>> not the actual discussion...
>
> This might be helpful.
> https://marc.info/?l=linux-mm=152054772413234=4
Thank you, although that does not explain why the changes are
happening, only that you are introducing the new API so that you can
start converting users (which is the normal way of doing it, no?).
What I meant is the discussion that led to commit 1c8f422059ae5 itself
(which has a link inside, but is broken).
>> I am out of the loop on these mm changes, so please indulge me, but:
>>
>> * Why is there no documentation on vmf_insert_page() while
>> vm_insert_page() had it? (specially since it seems you want to remove
>> vm_insert_page()).
>
> The plan is to convert vm_insert_{page,pfn,mixed} to
> vmf_insert_{page,pfn,mixed}. As a good intermediate
> steps inline wrapper vmf_insert_{pfn,page,mixed} are
> introduced. After all the drivers converted, we will convert
> vm_insert_page to vmf_insert_page and remove the inline
> wrapper and update the document at the same time.
Yeah, that is what 1c8f422059ae5 ("mm: change return type to
vm_fault_t") seems to say at the end (thanks for clarifying).
Still, that does not explain why the documentation was not added at
the same time as soon the new API is introduced. I don't see how it
matters that they are wrappers.
Actually, I think the wrappers should have been the final functions
already in memory.c, their declarations in mm.h, etc. That way you
would minimize the code changes later on: you would be only removing
dead code, rather than changing code again. Even if you forward the
calls for the moment, it would have been a much smaller change later
on.
>
>>
>> * Shouldn't we have a simple remap_page() or remap_kernel_page() to
>> fit this use case and avoid that dance? (another driver in auxdisplay
>> will require the same change, and I guess others in the kernel as
>> well).
>
>
> There are few drivers similar like auxdisplay where straight forward
> conversion from vm_insert_page to vmf_insert_page is not possible.
>
> So I mapped the kernel memory to user vma using remap_pfn_range
> and remove vm_insert_page in this driver.
>
> Other way, is to replace vm_insert_page with vmf_insert_page() and
> then convert VM_FAULT_CODE back to errno. But as part of vm_fault_t
> migration we have already removed/cleanup most the errno to VM_FAULT_CODE
> mapping from drivers. So I prefer not to take this option.
>
> Third, we can introduce a similar API like vm_insert_page say,
> vm_insert_kmem_page() and use it for same scenarios like this.
Yep, I think that is the best, unless there are only a couple of users
and you think nobody should be using it in the future.
Cheers,
Miguel
Re: [PATCH] auxdisplay/cfag12864bfb.c: Replace vm_insert_page
On Thu, Sep 20, 2018 at 10:23 PM, Souptick Joarder wrote:
> There is a plan to remove vm_insert_page permanently
> and replace it with new API vmf_insert_page which will
> return vm_fault_t type. As part of it vm_insert_page
> is removed from this driver.
A link to the discussion/plan would be nice. The commit 1c8f422059ae5
("mm: change return type to vm_fault_t") explains a bit, but has a
broken link :( Googling for the stuff returns many of the patches, but
not the actual discussion...
> static int cfag12864bfb_mmap(struct fb_info *info, struct vm_area_struct
> *vma)
> {
> - return vm_insert_page(vma, vma->vm_start,
> - virt_to_page(cfag12864b_buffer));
> + struct page *page;
> + unsigned long size = vma->vm_end - vma->vm_start;
> +
> + page = virt_to_page(cfag12864b_buffer);
> + return remap_pfn_range(vma, vma->vm_start, page_to_pfn(page),
> + size, vma->vm_page_prot);
I am out of the loop on these mm changes, so please indulge me, but:
* Why is there no documentation on vmf_insert_page() while
vm_insert_page() had it? (specially since it seems you want to remove
vm_insert_page()).
* Shouldn't we have a simple remap_page() or remap_kernel_page() to
fit this use case and avoid that dance? (another driver in auxdisplay
will require the same change, and I guess others in the kernel as
well).
Thanks!
Cheers,
Miguel
Re: [PATCH] auxdisplay/cfag12864bfb.c: Replace vm_insert_page
On Thu, Sep 20, 2018 at 10:23 PM, Souptick Joarder wrote:
> There is a plan to remove vm_insert_page permanently
> and replace it with new API vmf_insert_page which will
> return vm_fault_t type. As part of it vm_insert_page
> is removed from this driver.
A link to the discussion/plan would be nice. The commit 1c8f422059ae5
("mm: change return type to vm_fault_t") explains a bit, but has a
broken link :( Googling for the stuff returns many of the patches, but
not the actual discussion...
> static int cfag12864bfb_mmap(struct fb_info *info, struct vm_area_struct
> *vma)
> {
> - return vm_insert_page(vma, vma->vm_start,
> - virt_to_page(cfag12864b_buffer));
> + struct page *page;
> + unsigned long size = vma->vm_end - vma->vm_start;
> +
> + page = virt_to_page(cfag12864b_buffer);
> + return remap_pfn_range(vma, vma->vm_start, page_to_pfn(page),
> + size, vma->vm_page_prot);
I am out of the loop on these mm changes, so please indulge me, but:
* Why is there no documentation on vmf_insert_page() while
vm_insert_page() had it? (specially since it seems you want to remove
vm_insert_page()).
* Shouldn't we have a simple remap_page() or remap_kernel_page() to
fit this use case and avoid that dance? (another driver in auxdisplay
will require the same change, and I guess others in the kernel as
well).
Thanks!
Cheers,
Miguel
Re: linux-next: disabling -Wstringop-truncation
Hi Stephen, Andi, (Linus), On Fri, Aug 31, 2018 at 3:17 AM, Linus Torvalds wrote: > On Thu, Aug 30, 2018 at 3:07 PM Stephen Rothwell > wrote: >> >> I am now mainly using gcc v8.2 for my builds and -Wstringop-truncation >> causes so many warnings that I am sure to miss others, so I have >> applied the below to my fixes tree until the noise reduces. > > Applied. > > If people want the warning with W=xyz, then this is still the correct > patch, and we should add a line to re-enable it in > scripts/Makefile.extrawarn. For reference, I am sending the v5 of the Compiler Attributes series with the __nonstring patches appended on top, and I am adding the warning back as well, on W=1. I think W=1 is better: there are not that many warnings compared to those in W=2. AFAIK, it is relatively easy to achieve a clean W=1 (and most code should at least try be W=1 clean, no?). Also, note that now people can use __nonstring to suppress that warning (if applicable); and that using properly __nonstring can detect even more problems (i.e. adding __nonstring may trigger additional warnings, e.g. -Wstringop-overflow); so I think we should encourage the use of __nonstring. Cheers, Miguel
Re: linux-next: disabling -Wstringop-truncation
Hi Stephen, Andi, (Linus), On Fri, Aug 31, 2018 at 3:17 AM, Linus Torvalds wrote: > On Thu, Aug 30, 2018 at 3:07 PM Stephen Rothwell > wrote: >> >> I am now mainly using gcc v8.2 for my builds and -Wstringop-truncation >> causes so many warnings that I am sure to miss others, so I have >> applied the below to my fixes tree until the noise reduces. > > Applied. > > If people want the warning with W=xyz, then this is still the correct > patch, and we should add a line to re-enable it in > scripts/Makefile.extrawarn. For reference, I am sending the v5 of the Compiler Attributes series with the __nonstring patches appended on top, and I am adding the warning back as well, on W=1. I think W=1 is better: there are not that many warnings compared to those in W=2. AFAIK, it is relatively easy to achieve a clean W=1 (and most code should at least try be W=1 clean, no?). Also, note that now people can use __nonstring to suppress that warning (if applicable); and that using properly __nonstring can detect even more problems (i.e. adding __nonstring may trigger additional warnings, e.g. -Wstringop-overflow); so I think we should encourage the use of __nonstring. Cheers, Miguel
Re: [PATCH v2 0/2] Compiler Attributes: (naked only, for v4.19)
On Thu, Sep 20, 2018 at 3:59 PM, Greg Kroah-Hartman 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
Re: [PATCH v2 0/2] Compiler Attributes: (naked only, for v4.19)
On Thu, Sep 20, 2018 at 3:59 PM, Greg Kroah-Hartman 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
Re: [PATCH v2 2/2] Compiler Attributes: naked can be shared
On Thu, Sep 20, 2018 at 9:36 AM, Dominique Martinet 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
On Thu, Sep 20, 2018 at 9:36 AM, Dominique Martinet 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
On Thu, Sep 20, 2018 at 9:22 AM, Greg Kroah-Hartman 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 2/2] Compiler Attributes: naked can be shared
On Thu, Sep 20, 2018 at 9:22 AM, Greg Kroah-Hartman 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 2/2] Compiler Attributes: naked can be shared
Hi Dominique,
On Thu, Sep 20, 2018 at 1:05 AM, Dominique Martinet
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
Hi Dominique,
On Thu, Sep 20, 2018 at 1:05 AM, Dominique Martinet
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
On Wed, Sep 19, 2018 at 11:14 PM, Greg Kroah-Hartman 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
On Wed, Sep 19, 2018 at 11:14 PM, Greg Kroah-Hartman 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
Hi Greg,
On Tue, Sep 18, 2018 at 7:34 PM, Greg Kroah-Hartman
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
Hi Greg,
On Tue, Sep 18, 2018 at 7:34 PM, Greg Kroah-Hartman
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
[PATCH v2 2/2] Compiler Attributes: naked can be shared
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
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Suggested-by: Arnd Bergmann
Tested-by: Stefan Agner
Reviewed-by: Stefan Agner
Reviewed-by: Luc Van Oostenryck
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
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
[PATCH v2 2/2] Compiler Attributes: naked can be shared
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
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Suggested-by: Arnd Bergmann
Tested-by: Stefan Agner
Reviewed-by: Stefan Agner
Reviewed-by: Luc Van Oostenryck
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
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
[PATCH v2 1/2] Compiler Attributes: naked was fixed in gcc 4.6
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
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Tested-by: Stefan Agner
Reviewed-by: Stefan Agner
Reviewed-by: Luc Van Oostenryck
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
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 1/2] Compiler Attributes: naked was fixed in gcc 4.6
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
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Tested-by: Stefan Agner
Reviewed-by: Stefan Agner
Reviewed-by: Luc Van Oostenryck
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
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 0/2] Compiler Attributes: (naked only, for v4.19)
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 Cc: Eli Friedman Cc: Christopher Li Cc: Kees Cook Cc: Ingo Molnar Cc: Geert Uytterhoeven Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Masahiro Yamada Cc: Joe Perches Cc: Dominique Martinet Cc: Stefan Agner Cc: Luc Van Oostenryck Cc: Nick Desaulniers Cc: Linus Torvalds Cc: linux-spa...@vger.kernel.org Signed-off-by: Miguel Ojeda 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 0/2] Compiler Attributes: (naked only, for v4.19)
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 Cc: Eli Friedman Cc: Christopher Li Cc: Kees Cook Cc: Ingo Molnar Cc: Geert Uytterhoeven Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Masahiro Yamada Cc: Joe Perches Cc: Dominique Martinet Cc: Stefan Agner Cc: Luc Van Oostenryck Cc: Nick Desaulniers Cc: Linus Torvalds Cc: linux-spa...@vger.kernel.org Signed-off-by: Miguel Ojeda 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
Re: [PATCH 2/2] Compiler Attributes: naked can be shared
On Tue, Sep 18, 2018 at 9:30 AM, Greg Kroah-Hartman
wrote:
> On Mon, Sep 17, 2018 at 08:19:50PM -0700, Stefan Agner wrote:
>> On 13.09.2018 16:59, 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").
>> >
>> > Cc: Stefan Agner
>> > Cc: Rasmus Villemoes
>> > Cc: Eli Friedman
>> > Cc: Christopher Li
>> > Cc: Kees Cook
>> > Cc: Ingo Molnar
>> > Cc: Geert Uytterhoeven
>> > Cc: Greg Kroah-Hartman
>> > Cc: Masahiro Yamada
>> > Cc: Joe Perches
>> > Cc: Dominique Martinet
>> > Cc: Linus Torvalds
>> > Cc: linux-spa...@vger.kernel.org
>> > Suggested-by: Arnd Bergmann
>> > Reviewed-by: Luc Van Oostenryck
>> > Reviewed-by: Nick Desaulniers
>> > Signed-off-by: Miguel Ojeda
>>
>> Reviewed-by: Stefan Agner
>>
>> Ideally this two patches should go into v4.19. Not sure which path the patch
>> should take, MAINTAINERS is not quite clear about that. This fixes
>> 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
>> exclusive") for ARM32, which has been merged directly by
>> Linus, afaict...
>
> Good point, can you resend these to me with all of the acks added so
> far?
Sure (I think only Stefan is the added ack, though).
Cheers,
Miguel
Re: [PATCH 2/2] Compiler Attributes: naked can be shared
On Tue, Sep 18, 2018 at 9:30 AM, Greg Kroah-Hartman
wrote:
> On Mon, Sep 17, 2018 at 08:19:50PM -0700, Stefan Agner wrote:
>> On 13.09.2018 16:59, 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").
>> >
>> > Cc: Stefan Agner
>> > Cc: Rasmus Villemoes
>> > Cc: Eli Friedman
>> > Cc: Christopher Li
>> > Cc: Kees Cook
>> > Cc: Ingo Molnar
>> > Cc: Geert Uytterhoeven
>> > Cc: Greg Kroah-Hartman
>> > Cc: Masahiro Yamada
>> > Cc: Joe Perches
>> > Cc: Dominique Martinet
>> > Cc: Linus Torvalds
>> > Cc: linux-spa...@vger.kernel.org
>> > Suggested-by: Arnd Bergmann
>> > Reviewed-by: Luc Van Oostenryck
>> > Reviewed-by: Nick Desaulniers
>> > Signed-off-by: Miguel Ojeda
>>
>> Reviewed-by: Stefan Agner
>>
>> Ideally this two patches should go into v4.19. Not sure which path the patch
>> should take, MAINTAINERS is not quite clear about that. This fixes
>> 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
>> exclusive") for ARM32, which has been merged directly by
>> Linus, afaict...
>
> Good point, can you resend these to me with all of the acks added so
> far?
Sure (I think only Stefan is the added ack, though).
Cheers,
Miguel
Re: [PATCH] scripts: unifdef: fix stringop-truncation warning
On Fri, Sep 14, 2018 at 7:12 PM, Miguel Ojeda
wrote:
> Hi Harshit,
Another two nitpicks:
* Since this is the second version of the patch, write "[PATCH v2]"
in the title instead; that way people know you have sent an earlier
version. On top of that, you should explain the differences between
each version after the "---" line (i.e. the part that does not belong
to the commit message).
* You may want to write down who you CC'd (e.g. Tony and the stable
mailing list) in "Cc: ..." lines before the Signed-off-by line.
Thanks for the patch!
Hope that helps,
Miguel
>
> On Fri, Sep 14, 2018 at 12:44 PM, Harshit Jain
> wrote:
>> From: Harshit Jain
>>
>> * This commit resolves the following warning when the mainline kernel is
>> build with the android environment.
>
> Typo: built
>
>>
>> -> warning :->
>> https://gist.github.com/dev-harsh1998/757427b16a58f5498db3d87212a9651b
>
> Try to avoid links to external pages (specially if they are only 2 lines :-).
>
>>
>> * This warning is persistant in all the currently maintained android kernel
>> i.e 3.18, 4.4, 4.9, 4.14.
>
> Typos: persistent, kernels, i.e.
>
>>
>> Signed-off-by: Harshit Jain
>> ---
>> scripts/unifdef.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/unifdef.c b/scripts/unifdef.c
>> index 7493c0ee51cc..4ce008eda362 100644
>> --- a/scripts/unifdef.c
>> +++ b/scripts/unifdef.c
>> @@ -395,8 +395,8 @@ usage(void)
>> * When we have processed a group that starts off with a known-false
>> * #if/#elif sequence (which has therefore been deleted) followed by a
>> * #elif that we don't understand and therefore must keep, we edit the
>> - * latter into a #if to keep the nesting correct. We use strncpy() to
>> - * overwrite the 4 byte token "elif" with "if " without a '\0' byte.
>> + * latter into a #if to keep the nesting correct. We use the memcpy()
>> + * from the string header overwrite the 4 byte token "elif" with "if ".
>> *
>> * When we find a true #elif in a group, the following block will
>> * always be kept and the rest of the sequence after the next #elif or
>> @@ -450,7 +450,7 @@ static void Idrop (void) { Fdrop(); ignoreon(); }
>> static void Itrue (void) { Ftrue(); ignoreon(); }
>> static void Ifalse(void) { Ffalse(); ignoreon(); }
>> /* modify this line */
>> -static void Mpass (void) { strncpy(keyword, "if ", 4); Pelif(); }
>> +static void Mpass (void) { memcpy(keyword, "if ", 4); Pelif(); }
>
> Seems like keyword shouldn't use __attribute__((nonstring)), so this
> looks right.
>
> Cheers,
> Miguel
>
>> static void Mtrue (void) { keywordedit("else"); state(IS_TRUE_MIDDLE); }
>> static void Melif (void) { keywordedit("endif"); state(IS_FALSE_TRAILER); }
>> static void Melse (void) { keywordedit("endif"); state(IS_FALSE_ELSE); }
>> --
>> 2.18.0
>>
Re: [PATCH] scripts: unifdef: fix stringop-truncation warning
On Fri, Sep 14, 2018 at 7:12 PM, Miguel Ojeda
wrote:
> Hi Harshit,
Another two nitpicks:
* Since this is the second version of the patch, write "[PATCH v2]"
in the title instead; that way people know you have sent an earlier
version. On top of that, you should explain the differences between
each version after the "---" line (i.e. the part that does not belong
to the commit message).
* You may want to write down who you CC'd (e.g. Tony and the stable
mailing list) in "Cc: ..." lines before the Signed-off-by line.
Thanks for the patch!
Hope that helps,
Miguel
>
> On Fri, Sep 14, 2018 at 12:44 PM, Harshit Jain
> wrote:
>> From: Harshit Jain
>>
>> * This commit resolves the following warning when the mainline kernel is
>> build with the android environment.
>
> Typo: built
>
>>
>> -> warning :->
>> https://gist.github.com/dev-harsh1998/757427b16a58f5498db3d87212a9651b
>
> Try to avoid links to external pages (specially if they are only 2 lines :-).
>
>>
>> * This warning is persistant in all the currently maintained android kernel
>> i.e 3.18, 4.4, 4.9, 4.14.
>
> Typos: persistent, kernels, i.e.
>
>>
>> Signed-off-by: Harshit Jain
>> ---
>> scripts/unifdef.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/unifdef.c b/scripts/unifdef.c
>> index 7493c0ee51cc..4ce008eda362 100644
>> --- a/scripts/unifdef.c
>> +++ b/scripts/unifdef.c
>> @@ -395,8 +395,8 @@ usage(void)
>> * When we have processed a group that starts off with a known-false
>> * #if/#elif sequence (which has therefore been deleted) followed by a
>> * #elif that we don't understand and therefore must keep, we edit the
>> - * latter into a #if to keep the nesting correct. We use strncpy() to
>> - * overwrite the 4 byte token "elif" with "if " without a '\0' byte.
>> + * latter into a #if to keep the nesting correct. We use the memcpy()
>> + * from the string header overwrite the 4 byte token "elif" with "if ".
>> *
>> * When we find a true #elif in a group, the following block will
>> * always be kept and the rest of the sequence after the next #elif or
>> @@ -450,7 +450,7 @@ static void Idrop (void) { Fdrop(); ignoreon(); }
>> static void Itrue (void) { Ftrue(); ignoreon(); }
>> static void Ifalse(void) { Ffalse(); ignoreon(); }
>> /* modify this line */
>> -static void Mpass (void) { strncpy(keyword, "if ", 4); Pelif(); }
>> +static void Mpass (void) { memcpy(keyword, "if ", 4); Pelif(); }
>
> Seems like keyword shouldn't use __attribute__((nonstring)), so this
> looks right.
>
> Cheers,
> Miguel
>
>> static void Mtrue (void) { keywordedit("else"); state(IS_TRUE_MIDDLE); }
>> static void Melif (void) { keywordedit("endif"); state(IS_FALSE_TRAILER); }
>> static void Melse (void) { keywordedit("endif"); state(IS_FALSE_ELSE); }
>> --
>> 2.18.0
>>
Re: [PATCH] scripts: unifdef: fix stringop-truncation warning
Hi Harshit,
On Fri, Sep 14, 2018 at 12:44 PM, Harshit Jain
wrote:
> From: Harshit Jain
>
> * This commit resolves the following warning when the mainline kernel is
> build with the android environment.
Typo: built
>
> -> warning :->
> https://gist.github.com/dev-harsh1998/757427b16a58f5498db3d87212a9651b
Try to avoid links to external pages (specially if they are only 2 lines :-).
>
> * This warning is persistant in all the currently maintained android kernel
> i.e 3.18, 4.4, 4.9, 4.14.
Typos: persistent, kernels, i.e.
>
> Signed-off-by: Harshit Jain
> ---
> scripts/unifdef.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/unifdef.c b/scripts/unifdef.c
> index 7493c0ee51cc..4ce008eda362 100644
> --- a/scripts/unifdef.c
> +++ b/scripts/unifdef.c
> @@ -395,8 +395,8 @@ usage(void)
> * When we have processed a group that starts off with a known-false
> * #if/#elif sequence (which has therefore been deleted) followed by a
> * #elif that we don't understand and therefore must keep, we edit the
> - * latter into a #if to keep the nesting correct. We use strncpy() to
> - * overwrite the 4 byte token "elif" with "if " without a '\0' byte.
> + * latter into a #if to keep the nesting correct. We use the memcpy()
> + * from the string header overwrite the 4 byte token "elif" with "if ".
> *
> * When we find a true #elif in a group, the following block will
> * always be kept and the rest of the sequence after the next #elif or
> @@ -450,7 +450,7 @@ static void Idrop (void) { Fdrop(); ignoreon(); }
> static void Itrue (void) { Ftrue(); ignoreon(); }
> static void Ifalse(void) { Ffalse(); ignoreon(); }
> /* modify this line */
> -static void Mpass (void) { strncpy(keyword, "if ", 4); Pelif(); }
> +static void Mpass (void) { memcpy(keyword, "if ", 4); Pelif(); }
Seems like keyword shouldn't use __attribute__((nonstring)), so this
looks right.
Cheers,
Miguel
> static void Mtrue (void) { keywordedit("else"); state(IS_TRUE_MIDDLE); }
> static void Melif (void) { keywordedit("endif"); state(IS_FALSE_TRAILER); }
> static void Melse (void) { keywordedit("endif"); state(IS_FALSE_ELSE); }
> --
> 2.18.0
>
Re: [PATCH] scripts: unifdef: fix stringop-truncation warning
Hi Harshit,
On Fri, Sep 14, 2018 at 12:44 PM, Harshit Jain
wrote:
> From: Harshit Jain
>
> * This commit resolves the following warning when the mainline kernel is
> build with the android environment.
Typo: built
>
> -> warning :->
> https://gist.github.com/dev-harsh1998/757427b16a58f5498db3d87212a9651b
Try to avoid links to external pages (specially if they are only 2 lines :-).
>
> * This warning is persistant in all the currently maintained android kernel
> i.e 3.18, 4.4, 4.9, 4.14.
Typos: persistent, kernels, i.e.
>
> Signed-off-by: Harshit Jain
> ---
> scripts/unifdef.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/unifdef.c b/scripts/unifdef.c
> index 7493c0ee51cc..4ce008eda362 100644
> --- a/scripts/unifdef.c
> +++ b/scripts/unifdef.c
> @@ -395,8 +395,8 @@ usage(void)
> * When we have processed a group that starts off with a known-false
> * #if/#elif sequence (which has therefore been deleted) followed by a
> * #elif that we don't understand and therefore must keep, we edit the
> - * latter into a #if to keep the nesting correct. We use strncpy() to
> - * overwrite the 4 byte token "elif" with "if " without a '\0' byte.
> + * latter into a #if to keep the nesting correct. We use the memcpy()
> + * from the string header overwrite the 4 byte token "elif" with "if ".
> *
> * When we find a true #elif in a group, the following block will
> * always be kept and the rest of the sequence after the next #elif or
> @@ -450,7 +450,7 @@ static void Idrop (void) { Fdrop(); ignoreon(); }
> static void Itrue (void) { Ftrue(); ignoreon(); }
> static void Ifalse(void) { Ffalse(); ignoreon(); }
> /* modify this line */
> -static void Mpass (void) { strncpy(keyword, "if ", 4); Pelif(); }
> +static void Mpass (void) { memcpy(keyword, "if ", 4); Pelif(); }
Seems like keyword shouldn't use __attribute__((nonstring)), so this
looks right.
Cheers,
Miguel
> static void Mtrue (void) { keywordedit("else"); state(IS_TRUE_MIDDLE); }
> static void Melif (void) { keywordedit("endif"); state(IS_FALSE_TRAILER); }
> static void Melse (void) { keywordedit("endif"); state(IS_FALSE_ELSE); }
> --
> 2.18.0
>
Re: [PATCH] sched/core: Fix compiling warring in smp=n case
Hi,
On Thu, Sep 13, 2018 at 11:55 AM, Dou Liyang wrote:
>
>
> At 08/10/2018 10:35 AM, Dou Liyang wrote:
>>
>> When compiling kernel with SMP disabled, the build warns with:
>>
>> kernel/sched/core.c: In function ‘update_rq_clock_task’:
>> kernel/sched/core.c:139:17: warning: unused variable ‘irq_delta’
>> [-Wunused-variable]
>>s64 steal = 0, irq_delta = 0;
>>
>> Fix this by revert the HAVE_SCHED_AVG_IRQ to
>>
>>defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
>> defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
>>
>> Fixes: 2e62c4743adc ("sched/fair: Remove #ifdefs from
>> scale_rt_capacity()")
>
>
> Hi,
>
> Miguel also found this warning. Can we pick it up now? ;-)
Also Zhong has just sent another patch for this.
Whenever this is picked up, feel free to add:
Acked-by: Miguel Ojeda
Cheers,
Miguel
Re: [PATCH] sched/core: Fix compiling warring in smp=n case
Hi,
On Thu, Sep 13, 2018 at 11:55 AM, Dou Liyang wrote:
>
>
> At 08/10/2018 10:35 AM, Dou Liyang wrote:
>>
>> When compiling kernel with SMP disabled, the build warns with:
>>
>> kernel/sched/core.c: In function ‘update_rq_clock_task’:
>> kernel/sched/core.c:139:17: warning: unused variable ‘irq_delta’
>> [-Wunused-variable]
>>s64 steal = 0, irq_delta = 0;
>>
>> Fix this by revert the HAVE_SCHED_AVG_IRQ to
>>
>>defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
>> defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
>>
>> Fixes: 2e62c4743adc ("sched/fair: Remove #ifdefs from
>> scale_rt_capacity()")
>
>
> Hi,
>
> Miguel also found this warning. Can we pick it up now? ;-)
Also Zhong has just sent another patch for this.
Whenever this is picked up, feel free to add:
Acked-by: Miguel Ojeda
Cheers,
Miguel
Re: [PATCH] sched: Fix a GCC compiled warning.
Hi Zhong, On Fri, Sep 14, 2018 at 4:49 PM, zhong jiang wrote: > Fix the following compile warning: > > kernel/sched/core.c: In function ‘update_rq_clock_task’: > kernel/sched/core.c:139:6: warning: unused variable ‘steal’ > [-Wunused-variable] > s64 steal = 0, irq_delta = 0; > Dou (and I, without having noticed Dou's) also sent another patch for this, see: https://lore.kernel.org/patchwork/patch/973402/ https://lore.kernel.org/patchwork/patch/983794/ Cheers, Miguel > Signed-off-by: zhong jiang > --- > kernel/sched/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 7577ce4..1003c1d 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -136,7 +136,7 @@ static void update_rq_clock_task(struct rq *rq, s64 delta) > * to sched_rt_avg_update. But I don't trust it... > */ > #if defined(CONFIG_IRQ_TIME_ACCOUNTING) || > defined(CONFIG_PARAVIRT_TIME_ACCOUNTING) > - s64 steal = 0, irq_delta = 0; > + s64 __maybe_unused steal = 0, irq_delta = 0; > #endif > #ifdef CONFIG_IRQ_TIME_ACCOUNTING > irq_delta = irq_time_read(cpu_of(rq)) - rq->prev_irq_time; > -- > 1.7.12.4 >
Re: [PATCH] sched: Fix a GCC compiled warning.
Hi Zhong, On Fri, Sep 14, 2018 at 4:49 PM, zhong jiang wrote: > Fix the following compile warning: > > kernel/sched/core.c: In function ‘update_rq_clock_task’: > kernel/sched/core.c:139:6: warning: unused variable ‘steal’ > [-Wunused-variable] > s64 steal = 0, irq_delta = 0; > Dou (and I, without having noticed Dou's) also sent another patch for this, see: https://lore.kernel.org/patchwork/patch/973402/ https://lore.kernel.org/patchwork/patch/983794/ Cheers, Miguel > Signed-off-by: zhong jiang > --- > kernel/sched/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 7577ce4..1003c1d 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -136,7 +136,7 @@ static void update_rq_clock_task(struct rq *rq, s64 delta) > * to sched_rt_avg_update. But I don't trust it... > */ > #if defined(CONFIG_IRQ_TIME_ACCOUNTING) || > defined(CONFIG_PARAVIRT_TIME_ACCOUNTING) > - s64 steal = 0, irq_delta = 0; > + s64 __maybe_unused steal = 0, irq_delta = 0; > #endif > #ifdef CONFIG_IRQ_TIME_ACCOUNTING > irq_delta = irq_time_read(cpu_of(rq)) - rq->prev_irq_time; > -- > 1.7.12.4 >
[PATCH 2/2] Compiler Attributes: naked can be shared
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: Stefan Agner
Cc: Rasmus Villemoes
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Suggested-by: Arnd Bergmann
Reviewed-by: Luc Van Oostenryck
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
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
[PATCH 1/2] Compiler Attributes: naked was fixed in gcc 4.6
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: Stefan Agner
Cc: Rasmus Villemoes
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Luc Van Oostenryck
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
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 2/2] Compiler Attributes: naked can be shared
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: Stefan Agner
Cc: Rasmus Villemoes
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Suggested-by: Arnd Bergmann
Reviewed-by: Luc Van Oostenryck
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
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
[PATCH 1/2] Compiler Attributes: naked was fixed in gcc 4.6
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: Stefan Agner
Cc: Rasmus Villemoes
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Luc Van Oostenryck
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
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 0/2] Compiler Attributes (naked only, for v4.19)
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. 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 0/2] Compiler Attributes (naked only, for v4.19)
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. 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
Re: [PATCH] include/linux/compiler-clang.h: define __naked
Hi again, On Wed, Sep 12, 2018 at 6:19 AM, Miguel Ojeda wrote: > > I will do a bigger one tomorrow or so and see if there are any > important differences. Regardless of what we do, I will send the > __naked patches separately as well (requested by Nick on GitHub). So I did a comparison with a full allyesconfig between -rc2 and -rc2 + v4. I find quite a lot binary differences, but not a single objdump -d difference (object file by object file) just by fixing UTS_RELEASE to the same value, so I guess that is a very good sign in case someone wants to pick the entire series sooner than expected. Regardless, I will send the separate __naked patches. I should note that, at some point in one of my allyesconfig builds I got this: UPD include/generated/compile.h CC init/version.o AR init/built-in.a AR built-in.a LD vmlinux.o MODPOST vmlinux.o ld: drivers/edac/amd64_edac.o(.text+0x200031d2): reloc against `__asan_load4_noabort': error 4 ld: final link failed: Nonrepresentable section on output Makefile:1035: recipe for target 'vmlinux' failed make: *** [vmlinux] Error 1 Running make (without cleaning) didn't reproduce it, and the other allyesconfig builds didn't suffer any problem either. Maybe my hardware is failing, but I wanted to let you know nevertheless. Cheers, Miguel
Re: [PATCH] include/linux/compiler-clang.h: define __naked
Hi again, On Wed, Sep 12, 2018 at 6:19 AM, Miguel Ojeda wrote: > > I will do a bigger one tomorrow or so and see if there are any > important differences. Regardless of what we do, I will send the > __naked patches separately as well (requested by Nick on GitHub). So I did a comparison with a full allyesconfig between -rc2 and -rc2 + v4. I find quite a lot binary differences, but not a single objdump -d difference (object file by object file) just by fixing UTS_RELEASE to the same value, so I guess that is a very good sign in case someone wants to pick the entire series sooner than expected. Regardless, I will send the separate __naked patches. I should note that, at some point in one of my allyesconfig builds I got this: UPD include/generated/compile.h CC init/version.o AR init/built-in.a AR built-in.a LD vmlinux.o MODPOST vmlinux.o ld: drivers/edac/amd64_edac.o(.text+0x200031d2): reloc against `__asan_load4_noabort': error 4 ld: final link failed: Nonrepresentable section on output Makefile:1035: recipe for target 'vmlinux' failed make: *** [vmlinux] Error 1 Running make (without cleaning) didn't reproduce it, and the other allyesconfig builds didn't suffer any problem either. Maybe my hardware is failing, but I wanted to let you know nevertheless. Cheers, Miguel
Re: [PATCH] include/linux/compiler-clang.h: define __naked
Hi Arnd, Nick, Stefan,
On Mon, Sep 10, 2018 at 2:14 PM, Arnd Bergmann wrote:
> On Mon, Sep 10, 2018 at 8:05 AM Stefan Agner wrote:
>>
>> ARM32 arch code uses the __naked attribute. This has previously been
>> defined in include/linux/compiler-gcc.h, which is no longer included
>> for Clang. Define __naked for Clang. Conservatively add all attributes
>> previously used (and supported by Clang).
>>
>> This fixes compile errors when building ARM32 using Clang:
>> arch/arm/mach-exynos/mcpm-exynos.c:193:13: error: variable has incomplete
>> type 'void'
>> static void __naked exynos_pm_power_up_setup(unsigned int affinity_level)
>> ^
>>
>> Fixes: 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
>> exclusive")
>> Signed-off-by: Stefan Agner
>
>> +/*
>> + * ARM32 is currently the only user of __naked supported by Clang. Follow
>> + * gcc: Do not trace naked functions and make sure they don't get inlined.
>> + */
>> +#define __naked __attribute__((naked)) noinline notrace
>> +
>
> Please see patches 5 and 6 of the series that Miguel posted:
>
> https://lore.kernel.org/lkml/20180908212459.19736-6-miguel.ojeda.sando...@gmail.com/
>
> I suppose we want the patch to fix clang build as soon as possible though,
> and follow up with the cleanup for the next merge window, right?
Not sure what the plans of Linus et. al. are, if they have any; but
that would be a safe bet.
In case they want to speed this up and put the entire series into
v4.19 (instead of the two patches), I have done a binary & objdump
diff between -rc2 and v4 (based on -rc2) on all object files (with
UTS_RELEASE fixed to avoid some differences).
In a x86_64 tinyconfig with gcc 7.3, the differences I found are:
$ ./compare.py linux-rc2 linux-v4
[2018-09-12 06:16:39,483] [INFO] [arch/x86/boot/compressed/piggy.o]
Binary diff (use 'bash -c "cmp
linux-rc2/arch/x86/boot/compressed/piggy.o
linux-v4/arch/x86/boot/compressed/piggy.o"' to replicate)
[2018-09-12 06:16:39,606] [INFO] [arch/x86/boot/header.o] Binary diff
(use 'bash -c "cmp linux-rc2/arch/x86/boot/header.o
linux-v4/arch/x86/boot/header.o"' to replicate)
[2018-09-12 06:16:39,659] [INFO] [arch/x86/boot/version.o] Binary diff
(use 'bash -c "cmp linux-rc2/arch/x86/boot/version.o
linux-v4/arch/x86/boot/version.o"' to replicate)
[2018-09-12 06:16:40,483] [INFO] [init/version.o] Binary diff (use
'bash -c "cmp linux-rc2/init/version.o linux-v4/init/version.o"' to
replicate)
I will do a bigger one tomorrow or so and see if there are any
important differences. Regardless of what we do, I will send the
__naked patches separately as well (requested by Nick on GitHub).
Cheers,
Miguel
Re: [PATCH] include/linux/compiler-clang.h: define __naked
Hi Arnd, Nick, Stefan,
On Mon, Sep 10, 2018 at 2:14 PM, Arnd Bergmann wrote:
> On Mon, Sep 10, 2018 at 8:05 AM Stefan Agner wrote:
>>
>> ARM32 arch code uses the __naked attribute. This has previously been
>> defined in include/linux/compiler-gcc.h, which is no longer included
>> for Clang. Define __naked for Clang. Conservatively add all attributes
>> previously used (and supported by Clang).
>>
>> This fixes compile errors when building ARM32 using Clang:
>> arch/arm/mach-exynos/mcpm-exynos.c:193:13: error: variable has incomplete
>> type 'void'
>> static void __naked exynos_pm_power_up_setup(unsigned int affinity_level)
>> ^
>>
>> Fixes: 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
>> exclusive")
>> Signed-off-by: Stefan Agner
>
>> +/*
>> + * ARM32 is currently the only user of __naked supported by Clang. Follow
>> + * gcc: Do not trace naked functions and make sure they don't get inlined.
>> + */
>> +#define __naked __attribute__((naked)) noinline notrace
>> +
>
> Please see patches 5 and 6 of the series that Miguel posted:
>
> https://lore.kernel.org/lkml/20180908212459.19736-6-miguel.ojeda.sando...@gmail.com/
>
> I suppose we want the patch to fix clang build as soon as possible though,
> and follow up with the cleanup for the next merge window, right?
Not sure what the plans of Linus et. al. are, if they have any; but
that would be a safe bet.
In case they want to speed this up and put the entire series into
v4.19 (instead of the two patches), I have done a binary & objdump
diff between -rc2 and v4 (based on -rc2) on all object files (with
UTS_RELEASE fixed to avoid some differences).
In a x86_64 tinyconfig with gcc 7.3, the differences I found are:
$ ./compare.py linux-rc2 linux-v4
[2018-09-12 06:16:39,483] [INFO] [arch/x86/boot/compressed/piggy.o]
Binary diff (use 'bash -c "cmp
linux-rc2/arch/x86/boot/compressed/piggy.o
linux-v4/arch/x86/boot/compressed/piggy.o"' to replicate)
[2018-09-12 06:16:39,606] [INFO] [arch/x86/boot/header.o] Binary diff
(use 'bash -c "cmp linux-rc2/arch/x86/boot/header.o
linux-v4/arch/x86/boot/header.o"' to replicate)
[2018-09-12 06:16:39,659] [INFO] [arch/x86/boot/version.o] Binary diff
(use 'bash -c "cmp linux-rc2/arch/x86/boot/version.o
linux-v4/arch/x86/boot/version.o"' to replicate)
[2018-09-12 06:16:40,483] [INFO] [init/version.o] Binary diff (use
'bash -c "cmp linux-rc2/init/version.o linux-v4/init/version.o"' to
replicate)
I will do a bigger one tomorrow or so and see if there are any
important differences. Regardless of what we do, I will send the
__naked patches separately as well (requested by Nick on GitHub).
Cheers,
Miguel
Re: [PATCH] kernel/sched/core.c: Avoid unused variable on non-SMP configs
Hi Vincent, On Mon, Sep 10, 2018 at 9:00 AM, Vincent Guittot wrote: > On Sun, 9 Sep 2018 at 19:00, Miguel Ojeda > wrote: >> >> On Sun, Sep 9, 2018 at 6:45 PM, Borislav Petkov wrote: >> > On Sun, Sep 09, 2018 at 06:36:01PM +0200, Miguel Ojeda wrote: >> >> No, you get a different warning depending on whether you have enabled >> >> CONFIG_PARAVIRT_TIME_ACCOUNTING or CONFIG_IRQ_TIME_ACCOUNTING. >> > >> > Ok. >> > >> > Still, adding __maybe_unused to both (or writing it before the name, >> > whatever works!) and dropping the ifdeffery is still better for >> > readability's sake than having more ifdeffery, IMO. >> >> Agreed, it is quite confusing already. I tried to keep the style of >> the code, but Ingo/Peter might prefer the cleanup. Let's see... > > FYI, another patch has already been sent for this warning > https://lkml.org/lkml/2018/8/10/22 Indeed -- sorry, I didn't notice. The patches are different in behavior, though; is the block there supposed to be there in non-SMP cases? (I guess so, since originally it was there, but asking just in case). Cheers, Miguel
Re: [PATCH] kernel/sched/core.c: Avoid unused variable on non-SMP configs
Hi Vincent, On Mon, Sep 10, 2018 at 9:00 AM, Vincent Guittot wrote: > On Sun, 9 Sep 2018 at 19:00, Miguel Ojeda > wrote: >> >> On Sun, Sep 9, 2018 at 6:45 PM, Borislav Petkov wrote: >> > On Sun, Sep 09, 2018 at 06:36:01PM +0200, Miguel Ojeda wrote: >> >> No, you get a different warning depending on whether you have enabled >> >> CONFIG_PARAVIRT_TIME_ACCOUNTING or CONFIG_IRQ_TIME_ACCOUNTING. >> > >> > Ok. >> > >> > Still, adding __maybe_unused to both (or writing it before the name, >> > whatever works!) and dropping the ifdeffery is still better for >> > readability's sake than having more ifdeffery, IMO. >> >> Agreed, it is quite confusing already. I tried to keep the style of >> the code, but Ingo/Peter might prefer the cleanup. Let's see... > > FYI, another patch has already been sent for this warning > https://lkml.org/lkml/2018/8/10/22 Indeed -- sorry, I didn't notice. The patches are different in behavior, though; is the block there supposed to be there in non-SMP cases? (I guess so, since originally it was there, but asking just in case). Cheers, Miguel
Re: [PATCH] kernel/sched/core.c: Avoid unused variable on non-SMP configs
On Sun, Sep 9, 2018 at 6:45 PM, Borislav Petkov wrote: > On Sun, Sep 09, 2018 at 06:36:01PM +0200, Miguel Ojeda wrote: >> No, you get a different warning depending on whether you have enabled >> CONFIG_PARAVIRT_TIME_ACCOUNTING or CONFIG_IRQ_TIME_ACCOUNTING. > > Ok. > > Still, adding __maybe_unused to both (or writing it before the name, > whatever works!) and dropping the ifdeffery is still better for > readability's sake than having more ifdeffery, IMO. Agreed, it is quite confusing already. I tried to keep the style of the code, but Ingo/Peter might prefer the cleanup. Let's see... Cheers, Miguel
Re: [PATCH] kernel/sched/core.c: Avoid unused variable on non-SMP configs
On Sun, Sep 9, 2018 at 6:45 PM, Borislav Petkov wrote: > On Sun, Sep 09, 2018 at 06:36:01PM +0200, Miguel Ojeda wrote: >> No, you get a different warning depending on whether you have enabled >> CONFIG_PARAVIRT_TIME_ACCOUNTING or CONFIG_IRQ_TIME_ACCOUNTING. > > Ok. > > Still, adding __maybe_unused to both (or writing it before the name, > whatever works!) and dropping the ifdeffery is still better for > readability's sake than having more ifdeffery, IMO. Agreed, it is quite confusing already. I tried to keep the style of the code, but Ingo/Peter might prefer the cleanup. Let's see... Cheers, Miguel
Re: [PATCH] kernel/sched/core.c: Avoid unused variable on non-SMP configs
Hi Borislav, On Sun, Sep 9, 2018 at 6:06 PM, Borislav Petkov wrote: > On Sun, Sep 09, 2018 at 04:48:19PM +0200, Miguel Ojeda wrote: >> Indeed. But note that the attribute needs to be written before the >> variable name so that it applies to both variables (or write it twice) > > It warns only about 'steal' - not the other one. No, you get a different warning depending on whether you have enabled CONFIG_PARAVIRT_TIME_ACCOUNTING or CONFIG_IRQ_TIME_ACCOUNTING. In other words, !SMP && IRQ warns about "steal", while !SMP && PARAVIRT warns about "irq_delta" (the example warning I posted is for this last case). Cheers, Miguel
Re: [PATCH] kernel/sched/core.c: Avoid unused variable on non-SMP configs
Hi Borislav, On Sun, Sep 9, 2018 at 6:06 PM, Borislav Petkov wrote: > On Sun, Sep 09, 2018 at 04:48:19PM +0200, Miguel Ojeda wrote: >> Indeed. But note that the attribute needs to be written before the >> variable name so that it applies to both variables (or write it twice) > > It warns only about 'steal' - not the other one. No, you get a different warning depending on whether you have enabled CONFIG_PARAVIRT_TIME_ACCOUNTING or CONFIG_IRQ_TIME_ACCOUNTING. In other words, !SMP && IRQ warns about "steal", while !SMP && PARAVIRT warns about "irq_delta" (the example warning I posted is for this last case). Cheers, Miguel
[PATCH] arch/arm64/include/asm/jump_label.h: use asm_volatile_goto
All other uses of "asm goto" go through asm_volatile_goto
(including the arm version of the same file). For consistency,
use it here as well.
Cc: Nick Desaulniers
Cc: Catalin Marinas
Cc: Will Deacon
Cc: linux-arm-ker...@lists.infradead.org
Signed-off-by: Miguel Ojeda
---
arch/arm64/include/asm/jump_label.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/jump_label.h
b/arch/arm64/include/asm/jump_label.h
index 1b5e0e843c3a..7e2b3e360086 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -28,7 +28,7 @@
static __always_inline bool arch_static_branch(struct static_key *key, bool
branch)
{
- asm goto("1: nop\n\t"
+ asm_volatile_goto("1: nop\n\t"
".pushsection __jump_table, \"aw\"\n\t"
".align 3\n\t"
".quad 1b, %l[l_yes], %c0\n\t"
@@ -42,7 +42,7 @@ static __always_inline bool arch_static_branch(struct
static_key *key, bool bran
static __always_inline bool arch_static_branch_jump(struct static_key *key,
bool branch)
{
- asm goto("1: b %l[l_yes]\n\t"
+ asm_volatile_goto("1: b %l[l_yes]\n\t"
".pushsection __jump_table, \"aw\"\n\t"
".align 3\n\t"
".quad 1b, %l[l_yes], %c0\n\t"
--
2.17.1
[PATCH] arch/arm64/include/asm/jump_label.h: use asm_volatile_goto
All other uses of "asm goto" go through asm_volatile_goto
(including the arm version of the same file). For consistency,
use it here as well.
Cc: Nick Desaulniers
Cc: Catalin Marinas
Cc: Will Deacon
Cc: linux-arm-ker...@lists.infradead.org
Signed-off-by: Miguel Ojeda
---
arch/arm64/include/asm/jump_label.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/jump_label.h
b/arch/arm64/include/asm/jump_label.h
index 1b5e0e843c3a..7e2b3e360086 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -28,7 +28,7 @@
static __always_inline bool arch_static_branch(struct static_key *key, bool
branch)
{
- asm goto("1: nop\n\t"
+ asm_volatile_goto("1: nop\n\t"
".pushsection __jump_table, \"aw\"\n\t"
".align 3\n\t"
".quad 1b, %l[l_yes], %c0\n\t"
@@ -42,7 +42,7 @@ static __always_inline bool arch_static_branch(struct
static_key *key, bool bran
static __always_inline bool arch_static_branch_jump(struct static_key *key,
bool branch)
{
- asm goto("1: b %l[l_yes]\n\t"
+ asm_volatile_goto("1: b %l[l_yes]\n\t"
".pushsection __jump_table, \"aw\"\n\t"
".align 3\n\t"
".quad 1b, %l[l_yes], %c0\n\t"
--
2.17.1
Re: [PATCH] include/linux/compiler*.h: add version detection to asm_volatile_goto
Hi Nick, On Sat, Sep 8, 2018 at 12:21 AM, Nick Desaulniers wrote: > The comment above asm_volatile_goto mentions working around a GCC bug, > and links to a bug report that claims this has been fixed in newer > versions of GCC. Testing shows that this was resolved in GCC 4.8.2. > asm_volatile_goto should also be defined for other compilers that > support asm goto. > > Signed-off-by: Nick Desaulniers > --- > include/linux/compiler-gcc.h | 7 ++- > include/linux/compiler_types.h | 4 > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > index 763bbad1e258..149f411b4366 100644 > --- a/include/linux/compiler-gcc.h > +++ b/include/linux/compiler-gcc.h > @@ -171,7 +171,7 @@ > #endif > > /* > - * GCC 'asm goto' miscompiles certain code sequences: > + * GCC < 4.8.2 'asm goto' miscompiles certain code sequences: > * Nice catch. Indeed, it seems broken since 4.5.3 and fixed in every version after 4.8.1 (at least the ones in godbolt). Reviewed-by: Miguel Ojeda By the way, arch/arm64/include/asm/jump_label.h is the only place in the kernel using "asm goto" directly (e.g. the arm64 one isn't). Even if old gcc doesn't trigger the bug in that case, we should use the same everywhere for consistency, unless there is a special reason not to (not documented). I will send a patch. Cheers, Miguel
Re: [PATCH] include/linux/compiler*.h: add version detection to asm_volatile_goto
Hi Nick, On Sat, Sep 8, 2018 at 12:21 AM, Nick Desaulniers wrote: > The comment above asm_volatile_goto mentions working around a GCC bug, > and links to a bug report that claims this has been fixed in newer > versions of GCC. Testing shows that this was resolved in GCC 4.8.2. > asm_volatile_goto should also be defined for other compilers that > support asm goto. > > Signed-off-by: Nick Desaulniers > --- > include/linux/compiler-gcc.h | 7 ++- > include/linux/compiler_types.h | 4 > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > index 763bbad1e258..149f411b4366 100644 > --- a/include/linux/compiler-gcc.h > +++ b/include/linux/compiler-gcc.h > @@ -171,7 +171,7 @@ > #endif > > /* > - * GCC 'asm goto' miscompiles certain code sequences: > + * GCC < 4.8.2 'asm goto' miscompiles certain code sequences: > * Nice catch. Indeed, it seems broken since 4.5.3 and fixed in every version after 4.8.1 (at least the ones in godbolt). Reviewed-by: Miguel Ojeda By the way, arch/arm64/include/asm/jump_label.h is the only place in the kernel using "asm goto" directly (e.g. the arm64 one isn't). Even if old gcc doesn't trigger the bug in that case, we should use the same everywhere for consistency, unless there is a special reason not to (not documented). I will send a patch. Cheers, Miguel
Re: [PATCH] kernel/sched/core.c: Avoid unused variable on non-SMP configs
On Sun, Sep 9, 2018 at 11:36 AM, Borislav Petkov wrote: > > Alternatively, if you don't want to let the crazy ifdeffery in that > function grow even more, you can simply do: > > --- > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 625bc9897f62..1728743360d4 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -136,7 +136,7 @@ static void update_rq_clock_task(struct rq *rq, s64 delta) > * to sched_rt_avg_update. But I don't trust it... > */ > #if defined(CONFIG_IRQ_TIME_ACCOUNTING) || > defined(CONFIG_PARAVIRT_TIME_ACCOUNTING) > - s64 steal = 0, irq_delta = 0; > + s64 steal __maybe_unused = 0, irq_delta = 0; > #endif Indeed. But note that the attribute needs to be written before the variable name so that it applies to both variables (or write it twice) --- see https://godbolt.org/z/cwOeXZ Also, if we go that route, I would simply remove the #ifdef entirely. Cheers, Miguel
Re: [PATCH] kernel/sched/core.c: Avoid unused variable on non-SMP configs
On Sun, Sep 9, 2018 at 11:36 AM, Borislav Petkov wrote: > > Alternatively, if you don't want to let the crazy ifdeffery in that > function grow even more, you can simply do: > > --- > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 625bc9897f62..1728743360d4 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -136,7 +136,7 @@ static void update_rq_clock_task(struct rq *rq, s64 delta) > * to sched_rt_avg_update. But I don't trust it... > */ > #if defined(CONFIG_IRQ_TIME_ACCOUNTING) || > defined(CONFIG_PARAVIRT_TIME_ACCOUNTING) > - s64 steal = 0, irq_delta = 0; > + s64 steal __maybe_unused = 0, irq_delta = 0; > #endif Indeed. But note that the attribute needs to be written before the variable name so that it applies to both variables (or write it twice) --- see https://godbolt.org/z/cwOeXZ Also, if we go that route, I would simply remove the #ifdef entirely. Cheers, Miguel
[PATCH v4 05/13] Compiler Attributes: naked was fixed in gcc 4.6
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
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Nick Desaulniers
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Signed-off-by: Miguel Ojeda
---
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 76f4907ef707..4cd5e9264bce 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -77,14 +77,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 v4 10/13] Compiler Attributes: KENTRY used twice the "used" attribute
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 4030a2940d6b..17ee9165ca51 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -146,7 +146,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
extern typeof(sym) sym; \
static const unsigned long __kentry_##sym \
__used \
- __attribute__((__section__("___kentry" "+" #sym ), used)) \
+ __attribute__((__section__("___kentry" "+" #sym ))) \
= (unsigned long)
#endif
--
2.17.1
[PATCH v4 05/13] Compiler Attributes: naked was fixed in gcc 4.6
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
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Nick Desaulniers
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Signed-off-by: Miguel Ojeda
---
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 76f4907ef707..4cd5e9264bce 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -77,14 +77,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 v4 10/13] Compiler Attributes: KENTRY used twice the "used" attribute
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 4030a2940d6b..17ee9165ca51 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -146,7 +146,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
extern typeof(sym) sym; \
static const unsigned long __kentry_##sym \
__used \
- __attribute__((__section__("___kentry" "+" #sym ), used)) \
+ __attribute__((__section__("___kentry" "+" #sym ))) \
= (unsigned long)
#endif
--
2.17.1
[PATCH v4 04/13] Compiler Attributes: homogenize __must_be_array
Different definitions of __must_be_array:
* gcc: disabled for __CHECKER__
* clang: same definition as gcc's, but without __CHECKER__
* intel: the comment claims __builtin_types_compatible_p()
is unsupported; but icc seems to support it since 13.0.1
(released in 2012). See https://godbolt.org/z/S0l6QQ
Therefore, we can remove all of them and have a single definition
in compiler.h
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-clang.h | 1 -
include/linux/compiler-gcc.h | 7 ---
include/linux/compiler-intel.h | 3 ---
include/linux/compiler.h | 7 +++
4 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index d11cad61ba5c..fa9532f8d885 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -41,6 +41,5 @@
* compilers, like ICC.
*/
#define barrier() __asm__ __volatile__("" : : : "memory")
-#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
#define __assume_aligned(a, ...) \
__attribute__((__assume_aligned__(a, ## __VA_ARGS__)))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 371b6fa2d074..76f4907ef707 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -68,13 +68,6 @@
*/
#define uninitialized_var(x) x = x
-#ifdef __CHECKER__
-#define __must_be_array(a) 0
-#else
-/* [0] degrades to a pointer: a different type from an array */
-#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
-#endif
-
#ifdef RETPOLINE
#define __noretpoline __attribute__((__indirect_branch__("keep")))
#endif
diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index fef8bb3e53ef..6004b4588bd4 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -29,9 +29,6 @@
*/
#define OPTIMIZER_HIDE_VAR(var) barrier()
-/* Intel ECC compiler doesn't support __builtin_types_compatible_p() */
-#define __must_be_array(a) 0
-
#endif
/* icc has this, but it's called _bswap16 */
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index ec4a28bad2c6..165b1d5683ed 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -357,4 +357,11 @@ static inline void *offset_to_ptr(const int *off)
compiletime_assert(__native_word(t),\
"Need native word sized stores/loads for atomicity.")
+#ifdef __CHECKER__
+#define __must_be_array(a) 0
+#else
+/* [0] degrades to a pointer: a different type from an array */
+#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
+#endif
+
#endif /* __LINUX_COMPILER_H */
--
2.17.1
[PATCH v4 07/13] Compiler Attributes: remove unneeded sparse (__CHECKER__) tests
Sparse knows about a few more attributes now, so we can remove
the __CHECKER__ conditions from them (which, in turn, allow us
to move some of them later on to compiler_attributes.h).
* assume_aligned: since sparse's commit ffc860b ("sparse:
ignore __assume_aligned__ attribute"), included in 0.5.1
* error: since sparse's commit 0a04210 ("sparse: Add 'error'
to ignored attributes"), included in 0.5.0
* hotpatch: since sparse's commit 6043210 ("sparse/parse.c:
ignore hotpatch attribute"), included in 0.5.1
* warning: since sparse's commit 977365d ("Avoid "attribute
'warning': unknown attribute" warning"), included in 0.4.2
On top of that, __must_be_array does not need it either because:
* Even ancient versions of sparse do not have a problem
* BUILD_BUG_ON_ZERO() is currently disabled for __CHECKER__
Cc: Rasmus Villemoes
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Acked-by: Luc Van Oostenryck
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-gcc.h | 6 ++
include/linux/compiler.h | 4
include/linux/compiler_types.h | 2 +-
3 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 3b32bbfa5a49..1ca6a51cfaa9 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -76,14 +76,12 @@
#define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
-#ifndef __CHECKER__
#define __compiletime_warning(message) __attribute__((__warning__(message)))
#define __compiletime_error(message) __attribute__((__error__(message)))
-#ifdef LATENT_ENTROPY_PLUGIN
+#if defined(LATENT_ENTROPY_PLUGIN) && !defined(__CHECKER__)
#define __latent_entropy __attribute__((latent_entropy))
#endif
-#endif /* __CHECKER__ */
/*
* calling noreturn functions, __builtin_unreachable() and __builtin_trap()
@@ -131,7 +129,7 @@
/* gcc version specific checks */
-#if GCC_VERSION >= 40900 && !defined(__CHECKER__)
+#if GCC_VERSION >= 40900
/*
* __assume_aligned(n, k): Tell the optimizer that the returned
* pointer can be assumed to be k modulo n. The second argument is
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 165b1d5683ed..4030a2940d6b 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -357,11 +357,7 @@ static inline void *offset_to_ptr(const int *off)
compiletime_assert(__native_word(t),\
"Need native word sized stores/loads for atomicity.")
-#ifdef __CHECKER__
-#define __must_be_array(a) 0
-#else
/* [0] degrades to a pointer: a different type from an array */
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
-#endif
#endif /* __LINUX_COMPILER_H */
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 5ff9cda893f4..a3eceb3ad1b3 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -218,7 +218,7 @@ struct ftrace_likely_data {
#define __must_check
#endif
-#if defined(CC_USING_HOTPATCH) && !defined(__CHECKER__)
+#if defined(CC_USING_HOTPATCH)
#define notrace__attribute__((hotpatch(0, 0)))
#else
#define notrace
__attribute__((__no_instrument_function__))
--
2.17.1
[PATCH v4 03/13] Compiler Attributes: remove unneeded tests
Attributes const and always_inline have tests around them
which are unneeded, since they are supported by gcc >= 4.6,
clang >= 3 and icc >= 13. https://godbolt.org/z/DFPq37
In the case of gnu_inline, we do not need to test for
__GNUC_STDC_INLINE__ because, regardless of the current
inlining behavior, we can simply always force the old
GCC inlining behavior by using the attribute in all cases.
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler_types.h | 23 +++
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 2bc0f94df38e..83475515bc39 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -158,10 +158,6 @@ struct ftrace_likely_data {
(sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || \
sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
-#ifndef __attribute_const__
-#define __attribute_const____attribute__((__const__))
-#endif
-
#ifndef __noclone
#define __noclone
#endif
@@ -196,6 +192,7 @@ struct ftrace_likely_data {
* [...]
*/
#define __pure __attribute__((__pure__))
+#define __attribute_const____attribute__((__const__))
#define __aligned(x) __attribute__((__aligned__(x)))
#define __aligned_largest __attribute__((__aligned__))
#define __printf(a, b) __attribute__((__format__(printf, a, b)))
@@ -211,6 +208,8 @@ struct ftrace_likely_data {
#define __alias(symbol)__attribute__((__alias__(#symbol)))
#define __cold __attribute__((__cold__))
#define __section(S) __attribute__((__section__(#S)))
+#define __always_inlineinline
__attribute__((__always_inline__))
+#define __gnu_inline __attribute__((__gnu_inline__))
#ifdef CONFIG_ENABLE_MUST_CHECK
@@ -227,18 +226,6 @@ struct ftrace_likely_data {
#define __compiler_offsetof(a, b) __builtin_offsetof(a, b)
-/*
- * Feature detection for gnu_inline (gnu89 extern inline semantics). Either
- * __GNUC_STDC_INLINE__ is defined (not using gnu89 extern inline semantics,
- * and we opt in to the gnu89 semantics), or __GNUC_STDC_INLINE__ is not
- * defined so the gnu89 semantics are the default.
- */
-#ifdef __GNUC_STDC_INLINE__
-# define __gnu_inline __attribute__((__gnu_inline__))
-#else
-# define __gnu_inline
-#endif
-
/*
* Force always-inline if the user requests it so via the .config.
* GCC does not warn about unused static inline functions for
@@ -263,10 +250,6 @@ struct ftrace_likely_data {
#define __inline inline
#define noinline __attribute__((__noinline__))
-#ifndef __always_inline
-#define __always_inline inline __attribute__((__always_inline__))
-#endif
-
/*
* Rather then using noinline to prevent stack consumption, use
* noinline_for_stack instead. For documentation reasons.
--
2.17.1
[PATCH v4 04/13] Compiler Attributes: homogenize __must_be_array
Different definitions of __must_be_array:
* gcc: disabled for __CHECKER__
* clang: same definition as gcc's, but without __CHECKER__
* intel: the comment claims __builtin_types_compatible_p()
is unsupported; but icc seems to support it since 13.0.1
(released in 2012). See https://godbolt.org/z/S0l6QQ
Therefore, we can remove all of them and have a single definition
in compiler.h
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-clang.h | 1 -
include/linux/compiler-gcc.h | 7 ---
include/linux/compiler-intel.h | 3 ---
include/linux/compiler.h | 7 +++
4 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index d11cad61ba5c..fa9532f8d885 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -41,6 +41,5 @@
* compilers, like ICC.
*/
#define barrier() __asm__ __volatile__("" : : : "memory")
-#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
#define __assume_aligned(a, ...) \
__attribute__((__assume_aligned__(a, ## __VA_ARGS__)))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 371b6fa2d074..76f4907ef707 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -68,13 +68,6 @@
*/
#define uninitialized_var(x) x = x
-#ifdef __CHECKER__
-#define __must_be_array(a) 0
-#else
-/* [0] degrades to a pointer: a different type from an array */
-#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
-#endif
-
#ifdef RETPOLINE
#define __noretpoline __attribute__((__indirect_branch__("keep")))
#endif
diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index fef8bb3e53ef..6004b4588bd4 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -29,9 +29,6 @@
*/
#define OPTIMIZER_HIDE_VAR(var) barrier()
-/* Intel ECC compiler doesn't support __builtin_types_compatible_p() */
-#define __must_be_array(a) 0
-
#endif
/* icc has this, but it's called _bswap16 */
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index ec4a28bad2c6..165b1d5683ed 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -357,4 +357,11 @@ static inline void *offset_to_ptr(const int *off)
compiletime_assert(__native_word(t),\
"Need native word sized stores/loads for atomicity.")
+#ifdef __CHECKER__
+#define __must_be_array(a) 0
+#else
+/* [0] degrades to a pointer: a different type from an array */
+#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
+#endif
+
#endif /* __LINUX_COMPILER_H */
--
2.17.1
[PATCH v4 07/13] Compiler Attributes: remove unneeded sparse (__CHECKER__) tests
Sparse knows about a few more attributes now, so we can remove
the __CHECKER__ conditions from them (which, in turn, allow us
to move some of them later on to compiler_attributes.h).
* assume_aligned: since sparse's commit ffc860b ("sparse:
ignore __assume_aligned__ attribute"), included in 0.5.1
* error: since sparse's commit 0a04210 ("sparse: Add 'error'
to ignored attributes"), included in 0.5.0
* hotpatch: since sparse's commit 6043210 ("sparse/parse.c:
ignore hotpatch attribute"), included in 0.5.1
* warning: since sparse's commit 977365d ("Avoid "attribute
'warning': unknown attribute" warning"), included in 0.4.2
On top of that, __must_be_array does not need it either because:
* Even ancient versions of sparse do not have a problem
* BUILD_BUG_ON_ZERO() is currently disabled for __CHECKER__
Cc: Rasmus Villemoes
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Acked-by: Luc Van Oostenryck
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-gcc.h | 6 ++
include/linux/compiler.h | 4
include/linux/compiler_types.h | 2 +-
3 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 3b32bbfa5a49..1ca6a51cfaa9 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -76,14 +76,12 @@
#define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
-#ifndef __CHECKER__
#define __compiletime_warning(message) __attribute__((__warning__(message)))
#define __compiletime_error(message) __attribute__((__error__(message)))
-#ifdef LATENT_ENTROPY_PLUGIN
+#if defined(LATENT_ENTROPY_PLUGIN) && !defined(__CHECKER__)
#define __latent_entropy __attribute__((latent_entropy))
#endif
-#endif /* __CHECKER__ */
/*
* calling noreturn functions, __builtin_unreachable() and __builtin_trap()
@@ -131,7 +129,7 @@
/* gcc version specific checks */
-#if GCC_VERSION >= 40900 && !defined(__CHECKER__)
+#if GCC_VERSION >= 40900
/*
* __assume_aligned(n, k): Tell the optimizer that the returned
* pointer can be assumed to be k modulo n. The second argument is
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 165b1d5683ed..4030a2940d6b 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -357,11 +357,7 @@ static inline void *offset_to_ptr(const int *off)
compiletime_assert(__native_word(t),\
"Need native word sized stores/loads for atomicity.")
-#ifdef __CHECKER__
-#define __must_be_array(a) 0
-#else
/* [0] degrades to a pointer: a different type from an array */
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
-#endif
#endif /* __LINUX_COMPILER_H */
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 5ff9cda893f4..a3eceb3ad1b3 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -218,7 +218,7 @@ struct ftrace_likely_data {
#define __must_check
#endif
-#if defined(CC_USING_HOTPATCH) && !defined(__CHECKER__)
+#if defined(CC_USING_HOTPATCH)
#define notrace__attribute__((hotpatch(0, 0)))
#else
#define notrace
__attribute__((__no_instrument_function__))
--
2.17.1
[PATCH v4 03/13] Compiler Attributes: remove unneeded tests
Attributes const and always_inline have tests around them
which are unneeded, since they are supported by gcc >= 4.6,
clang >= 3 and icc >= 13. https://godbolt.org/z/DFPq37
In the case of gnu_inline, we do not need to test for
__GNUC_STDC_INLINE__ because, regardless of the current
inlining behavior, we can simply always force the old
GCC inlining behavior by using the attribute in all cases.
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler_types.h | 23 +++
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 2bc0f94df38e..83475515bc39 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -158,10 +158,6 @@ struct ftrace_likely_data {
(sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || \
sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
-#ifndef __attribute_const__
-#define __attribute_const____attribute__((__const__))
-#endif
-
#ifndef __noclone
#define __noclone
#endif
@@ -196,6 +192,7 @@ struct ftrace_likely_data {
* [...]
*/
#define __pure __attribute__((__pure__))
+#define __attribute_const____attribute__((__const__))
#define __aligned(x) __attribute__((__aligned__(x)))
#define __aligned_largest __attribute__((__aligned__))
#define __printf(a, b) __attribute__((__format__(printf, a, b)))
@@ -211,6 +208,8 @@ struct ftrace_likely_data {
#define __alias(symbol)__attribute__((__alias__(#symbol)))
#define __cold __attribute__((__cold__))
#define __section(S) __attribute__((__section__(#S)))
+#define __always_inlineinline
__attribute__((__always_inline__))
+#define __gnu_inline __attribute__((__gnu_inline__))
#ifdef CONFIG_ENABLE_MUST_CHECK
@@ -227,18 +226,6 @@ struct ftrace_likely_data {
#define __compiler_offsetof(a, b) __builtin_offsetof(a, b)
-/*
- * Feature detection for gnu_inline (gnu89 extern inline semantics). Either
- * __GNUC_STDC_INLINE__ is defined (not using gnu89 extern inline semantics,
- * and we opt in to the gnu89 semantics), or __GNUC_STDC_INLINE__ is not
- * defined so the gnu89 semantics are the default.
- */
-#ifdef __GNUC_STDC_INLINE__
-# define __gnu_inline __attribute__((__gnu_inline__))
-#else
-# define __gnu_inline
-#endif
-
/*
* Force always-inline if the user requests it so via the .config.
* GCC does not warn about unused static inline functions for
@@ -263,10 +250,6 @@ struct ftrace_likely_data {
#define __inline inline
#define noinline __attribute__((__noinline__))
-#ifndef __always_inline
-#define __always_inline inline __attribute__((__always_inline__))
-#endif
-
/*
* Rather then using noinline to prevent stack consumption, use
* noinline_for_stack instead. For documentation reasons.
--
2.17.1
[PATCH v4 09/13] Compiler Attributes: use feature checks instead of version checks
Instead of using version checks per-compiler to define (or not)
each attribute, use __has_attribute to test for them, following
the cleanup started with commit 815f0ddb346c
("include/linux/compiler*.h: make compiler-*.h mutually exclusive"),
which is supported on gcc >= 5, clang >= 2.9 and icc >= 17.
In the meantime, to support 4.6 <= gcc < 5, we implement
__has_attribute by hand.
All the attributes that can be unconditionally defined and directly
map to compiler attribute(s) (even if optional) have been moved
to a new file include/linux/compiler_attributes.h
In an effort to make the file as regular as possible, comments
stating the purpose of attributes have been removed. Instead,
links to the compiler docs have been added (i.e. to gcc and,
if available, to clang as well). In addition, they have been sorted.
Finally, if an attribute is optional (i.e. if it is guarded
by __has_attribute), the reason has been stated for future reference.
Cc: Rasmus Villemoes
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Nick Desaulniers
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Luc Van Oostenryck
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-clang.h | 4 -
include/linux/compiler-gcc.h| 51 --
include/linux/compiler-intel.h | 6 -
include/linux/compiler_attributes.h | 244
include/linux/compiler_types.h | 74 ++---
5 files changed, 254 insertions(+), 125 deletions(-)
create mode 100644 include/linux/compiler_attributes.h
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index fa9532f8d885..3e7dafb3ea80 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -21,8 +21,6 @@
#define __SANITIZE_ADDRESS__
#endif
-#define __no_sanitize_address __attribute__((__no_sanitize__("address")))
-
/*
* Not all versions of clang implement the the type-generic versions
* of the builtin overflow checkers. Fortunately, clang implements
@@ -41,5 +39,3 @@
* compilers, like ICC.
*/
#define barrier() __asm__ __volatile__("" : : : "memory")
-#define __assume_aligned(a, ...) \
- __attribute__((__assume_aligned__(a, ## __VA_ARGS__)))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 1ca6a51cfaa9..cfac027e1625 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -108,9 +108,6 @@
__builtin_unreachable();\
} while (0)
-/* Mark a function definition as prohibited from being cloned. */
-#define __noclone __attribute__((__noclone__, __optimize__("no-tracer")))
-
#if defined(RANDSTRUCT_PLUGIN) && !defined(__CHECKER__)
#define __randomize_layout __attribute__((randomize_layout))
#define __no_randomize_layout __attribute__((no_randomize_layout))
@@ -119,32 +116,6 @@
#define randomized_struct_fields_end } __randomize_layout;
#endif
-/*
- * When used with Link Time Optimization, gcc can optimize away C functions or
- * variables which are referenced only from assembly code. __visible tells the
- * optimizer that something else uses this function or variable, thus
preventing
- * this.
- */
-#define __visible __attribute__((__externally_visible__))
-
-/* gcc version specific checks */
-
-#if GCC_VERSION >= 40900
-/*
- * __assume_aligned(n, k): Tell the optimizer that the returned
- * pointer can be assumed to be k modulo n. The second argument is
- * optional (default 0), so we use a variadic macro to make the
- * shorthand.
- *
- * Beware: Do not apply this to functions which may return
- * ERR_PTRs. Also, it is probably unwise to apply it to functions
- * returning extra information in the low bits (but in that case the
- * compiler should see some alignment anyway, when the return value is
- * massaged by 'flags = ptr & 3; ptr &= ~3;').
- */
-#define __assume_aligned(a, ...) __attribute__((__assume_aligned__(a, ##
__VA_ARGS__)))
-#endif
-
/*
* GCC 'asm goto' miscompiles certain code sequences:
*
@@ -176,32 +147,10 @@
#define KASAN_ABI_VERSION 3
#endif
-#if GCC_VERSION >= 40902
-/*
- * Tell the compiler that address safety instrumentation (KASAN)
- * should not be applied to that function.
- * Conflicts with inlining: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
- */
-#define __no_sanitize_address __attribute__((__no_sanitize_address__))
-#endif
-
#if GCC_VERSION >= 50100
-/*
- * Mark structures as requiring designated initializers.
- * https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
- */
-#define __designated_init __attribute__((__designated_init__))
#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
#endif
-#if !defined(__noclone)
-#define __noclone /* not needed */
-#endif
-
-#if !defin
[PATCH v4 00/13] Compiler Attributes
The Compiler Attributes series is an effort to disentangle the include/linux/compiler*.h headers and bring them up to date. The main idea behind the series is to use feature checking macros (i.e. __has_attribute) instead of compiler version checks (e.g. GCC_VERSION), which are compiler-agnostic (so they can be shared, reducing the size of compiler-specific headers) and version-agnostic. Other related improvements have been performed in the headers as well, which on top of the use of __has_attribute it has amounted to a significant simplification of these headers (e.g. GCC_VERSION is now only guarding 4 non-attribute macros). This series should also help the efforts to support compiling the kernel with clang and icc. A fair amount of documentation and comments have also been added, clarified or removed; and the headers are now more readable, which should help kernel developers in general. The series was triggered due to the move to gcc >= 4.6. In turn, this series has also triggered Sparse to gain the ability to recognize __has_attribute on its own. You can also fetch it from: https://github.com/ojeda/linux/tree/compiler-attributes-v4 Enjoy! Cheers, Miguel Cc: Jonathan Corbet Cc: Rasmus Villemoes Cc: Luc Van Oostenryck Cc: Eli Friedman Cc: Christopher Li Cc: Kees Cook Cc: Ingo Molnar Cc: Geert Uytterhoeven Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Masahiro Yamada Cc: Joe Perches Cc: Dominique Martinet Cc: Nick Desaulniers Cc: Linus Torvalds Cc: linux-spa...@vger.kernel.org Cc: linux-...@vger.kernel.org Signed-off-by: Miguel Ojeda v3 -> v4 This time it is an important fix I found while doing randconfigs, plus the documentation sentence that Nick suggested and the MAINTAINERS file. Compile-tested for a while on (x86_64, gcc-7.3) on a few randconfs. New: * Add MAINTAINERS entry for the new file (compiler_attributes.h) so that we try to maintain that one as clean as possible. (if someone else wants to join, feel free!) Modified: * Fix inline macro __always_inline cannot be used in the inline macro, because people marking a function as __always_inline would expand to inline which in turn would expand back to __always_inline (under some configs), which would not be expanded again. Added a comment about this in the inline macro. Also added another comment about __always_inline in compiler_attributes.h to note that users expect to not have to write inline (as well as the attribute). We cannot simply remove "inline" there (which would solve the problem described above) because using the attribute is not enough for gcc. A function marked as always_inline seems to require to be marked as inline itself so that the attribute is applied: "warning: always_inline function might not be inlinable [-Wattributes]" From the gcc docs: "For functions declared inline, this attribute inlines the function independent of any restrictions that otherwise apply to inlining." See https://godbolt.org/z/LpzUPj for an example. From reviews: * Add sentence about feature-detection in Docs (Nick) Miguel Ojeda (13): Compiler Attributes: remove unused attributes Compiler Attributes: always use the extra-underscores syntax Compiler Attributes: remove unneeded tests Compiler Attributes: homogenize __must_be_array Compiler Attributes: naked was fixed in gcc 4.6 Compiler Attributes: naked can be shared Compiler Attributes: remove unneeded sparse (__CHECKER__) tests Compiler Attributes: add missing SPDX ID in compiler_types.h Compiler Attributes: use feature checks instead of version checks Compiler Attributes: KENTRY used twice the "used" attribute Compiler Attributes: remove uses of __attribute__ from compiler.h Compiler Attributes: add Doc/process/programming-language.rst Compiler Attributes: Add MAINTAINERS entry Documentation/process/index.rst | 1 + .../process/programming-language.rst | 45 MAINTAINERS | 5 + include/linux/compiler-clang.h| 5 - include/linux/compiler-gcc.h | 84 +- include/linux/compiler-intel.h| 9 - include/linux/compiler.h | 19 +- include/linux/compiler_attributes.h | 244 ++ include/linux/compiler_types.h| 105 ++-- 9 files changed, 329 insertions(+), 188 deletions(-) create mode 100644 Documentation/process/programming-language.rst create mode 100644 include/linux/compiler_attributes.h -- 2.17.1
[PATCH v4 08/13] Compiler Attributes: add missing SPDX ID in compiler_types.h
Cc: Rasmus Villemoes Cc: Luc Van Oostenryck Cc: Eli Friedman Cc: Christopher Li Cc: Kees Cook Cc: Ingo Molnar Cc: Geert Uytterhoeven Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Masahiro Yamada Cc: Joe Perches Cc: Dominique Martinet Cc: Nick Desaulniers Cc: Linus Torvalds Cc: linux-spa...@vger.kernel.org Signed-off-by: Miguel Ojeda --- include/linux/compiler_types.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index a3eceb3ad1b3..ed7c0e4a180e 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -1,3 +1,4 @@ +/* SPDX-License-Identifier: GPL-2.0 */ #ifndef __LINUX_COMPILER_TYPES_H #define __LINUX_COMPILER_TYPES_H -- 2.17.1
[PATCH v4 13/13] Compiler Attributes: Add MAINTAINERS entry
Cc: Eli Friedman Cc: Christopher Li Cc: Kees Cook Cc: Ingo Molnar Cc: Geert Uytterhoeven Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Masahiro Yamada Cc: Joe Perches Cc: Dominique Martinet Cc: Nick Desaulniers Cc: Linus Torvalds Signed-off-by: Miguel Ojeda --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9ad052aeac39..a7ef76d694d4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3721,6 +3721,11 @@ L: platform-driver-...@vger.kernel.org S: Maintained F: drivers/platform/x86/compal-laptop.c +COMPILER ATTRIBUTES +M: Miguel Ojeda +S: Maintained +F: include/linux/compiler_attributes.h + CONEXANT ACCESSRUNNER USB DRIVER L: accessrunner-gene...@lists.sourceforge.net W: http://accessrunner.sourceforge.net/ -- 2.17.1
[PATCH v4 02/13] Compiler Attributes: always use the extra-underscores syntax
The attribute syntax optionally allows to surround attribute names
with "__" in order to avoid collisions with macros of the same name
(see https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html).
This homogenizes all attributes to use the syntax with underscores.
While there are currently only a handful of cases of some TUs defining
macros like "error" which may collide with the attributes,
this should prevent futures surprises.
This has been done only for "standard" attributes supported by
the major compilers. In other words, those of third-party tools
(e.g. sparse, plugins...) have not been changed for the moment.
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-clang.h | 2 +-
include/linux/compiler-gcc.h | 14 ++--
include/linux/compiler-intel.h | 2 +-
include/linux/compiler.h | 8 +++
include/linux/compiler_types.h | 40 +-
5 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index b1ce500fe8b3..d11cad61ba5c 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -21,7 +21,7 @@
#define __SANITIZE_ADDRESS__
#endif
-#define __no_sanitize_address __attribute__((no_sanitize("address")))
+#define __no_sanitize_address __attribute__((__no_sanitize__("address")))
/*
* Not all versions of clang implement the the type-generic versions
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 0a2d06677d83..371b6fa2d074 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -76,7 +76,7 @@
#endif
#ifdef RETPOLINE
-#define __noretpoline __attribute__((indirect_branch("keep")))
+#define __noretpoline __attribute__((__indirect_branch__("keep")))
#endif
/*
@@ -91,15 +91,15 @@
* 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__)) noinline __noclone
notrace
#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
#define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
#ifndef __CHECKER__
-#define __compiletime_warning(message) __attribute__((warning(message)))
-#define __compiletime_error(message) __attribute__((error(message)))
+#define __compiletime_warning(message) __attribute__((__warning__(message)))
+#define __compiletime_error(message) __attribute__((__error__(message)))
#ifdef LATENT_ENTROPY_PLUGIN
#define __latent_entropy __attribute__((latent_entropy))
@@ -148,7 +148,7 @@
* optimizer that something else uses this function or variable, thus
preventing
* this.
*/
-#define __visible __attribute__((externally_visible))
+#define __visible __attribute__((__externally_visible__))
/* gcc version specific checks */
@@ -205,7 +205,7 @@
* should not be applied to that function.
* Conflicts with inlining: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
*/
-#define __no_sanitize_address __attribute__((no_sanitize_address))
+#define __no_sanitize_address __attribute__((__no_sanitize_address__))
#endif
#if GCC_VERSION >= 50100
@@ -213,7 +213,7 @@
* Mark structures as requiring designated initializers.
* https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
*/
-#define __designated_init __attribute__((designated_init))
+#define __designated_init __attribute__((__designated_init__))
#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
#endif
diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index 4c7f9befa9f6..fef8bb3e53ef 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -42,4 +42,4 @@
* and may be redefined here because they should not be shared with other
* compilers, like clang.
*/
-#define __visible __attribute__((externally_visible))
+#define __visible __attribute__((__externally_visible__))
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 7c0157d50964..ec4a28bad2c6 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -24,7 +24,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
long __r; \
static struct ftrace_likely_data\
__attribute__((__aligned__(4))) \
-
__attribute__((section("_ftrace_anno
[PATCH v4 01/13] Compiler Attributes: remove unused attributes
__optimize and __deprecate_for_modules are unused in
the whole kernel tree. Simply drop them.
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-gcc.h | 2 --
include/linux/compiler.h | 4
include/linux/compiler_types.h | 1 -
3 files changed, 7 deletions(-)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 763bbad1e258..0a2d06677d83 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -95,8 +95,6 @@
#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
-#define __optimize(level) __attribute__((__optimize__(level)))
-
#define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
#ifndef __CHECKER__
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 681d866efb1e..7c0157d50964 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -301,10 +301,6 @@ static inline void *offset_to_ptr(const int *off)
#endif /* __ASSEMBLY__ */
-#ifndef __optimize
-# define __optimize(level)
-#endif
-
/* Compile time object size, -1 for unknown */
#ifndef __compiletime_object_size
# define __compiletime_object_size(obj) -1
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 3525c179698c..b6534292ea33 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -108,7 +108,6 @@ struct ftrace_likely_data {
/* Don't. Just don't. */
#define __deprecated
-#define __deprecated_for_modules
#endif /* __KERNEL__ */
--
2.17.1
[PATCH v4 12/13] Compiler Attributes: add Doc/process/programming-language.rst
Cc: Jonathan Corbet Cc: Rasmus Villemoes Cc: Luc Van Oostenryck Cc: Eli Friedman Cc: Christopher Li Cc: Kees Cook Cc: Ingo Molnar Cc: Geert Uytterhoeven Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Masahiro Yamada Cc: Joe Perches Cc: Dominique Martinet Cc: Nick Desaulniers Cc: Linus Torvalds Cc: linux-spa...@vger.kernel.org Cc: linux-...@vger.kernel.org Signed-off-by: Miguel Ojeda --- Documentation/process/index.rst | 1 + .../process/programming-language.rst | 45 +++ 2 files changed, 46 insertions(+) create mode 100644 Documentation/process/programming-language.rst diff --git a/Documentation/process/index.rst b/Documentation/process/index.rst index 37bd0628b6ee..c56f24a22d2a 100644 --- a/Documentation/process/index.rst +++ b/Documentation/process/index.rst @@ -23,6 +23,7 @@ Below are the essential guides that every developer should read. code-of-conflict development-process submitting-patches + programming-language coding-style maintainer-pgp-guide email-clients diff --git a/Documentation/process/programming-language.rst b/Documentation/process/programming-language.rst new file mode 100644 index ..e5f5f065dc24 --- /dev/null +++ b/Documentation/process/programming-language.rst @@ -0,0 +1,45 @@ +.. _programming_language: + +Programming Language + + +The kernel is written in the C programming language [c-language]_. +More precisely, the kernel is typically compiled with ``gcc`` [gcc]_ +under ``-std=gnu89`` [gcc-c-dialect-options]_: the GNU dialect of ISO C90 +(including some C99 features). + +This dialect contains many extensions to the language [gnu-extensions]_, +and many of them are used within the kernel as a matter of course. + +There is some support for compiling the kernel with ``clang`` [clang]_ +and ``icc`` [icc]_ for several of the architectures, although at the time +of writing it is not completed, requiring third-party patches. + +Attributes +-- + +One of the common extensions used throughout the kernel are attributes +[gcc-attribute-syntax]_. Attributes allow to introduce +implementation-defined semantics to language entities (like variables, +functions or types) without having to make significant syntactic changes +to the language (e.g. adding a new keyword) [n2049]_. + +In some cases, attributes are optional (i.e. a compiler not supporting them +should still produce proper code, even if it is slower or does not perform +as many compile-time checks/diagnostics). + +The kernel defines pseudo-keywords (e.g. ``__pure``) instead of using +directly the GNU attribute syntax (e.g. ``__attribute__((__pure__))``) +in order to feature detect which ones can be used and/or to shorten the code. + +Please refer to ``include/linux/compiler_attributes.h`` for more information. + +.. [c-language] http://www.open-std.org/jtc1/sc22/wg14/www/standards +.. [gcc] https://gcc.gnu.org +.. [clang] https://clang.llvm.org +.. [icc] https://software.intel.com/en-us/c-compilers +.. [gcc-c-dialect-options] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html +.. [gnu-extensions] https://gcc.gnu.org/onlinedocs/gcc/C-Extensions.html +.. [gcc-attribute-syntax] https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html +.. [n2049] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2049.pdf + -- 2.17.1
[PATCH v4 09/13] Compiler Attributes: use feature checks instead of version checks
Instead of using version checks per-compiler to define (or not)
each attribute, use __has_attribute to test for them, following
the cleanup started with commit 815f0ddb346c
("include/linux/compiler*.h: make compiler-*.h mutually exclusive"),
which is supported on gcc >= 5, clang >= 2.9 and icc >= 17.
In the meantime, to support 4.6 <= gcc < 5, we implement
__has_attribute by hand.
All the attributes that can be unconditionally defined and directly
map to compiler attribute(s) (even if optional) have been moved
to a new file include/linux/compiler_attributes.h
In an effort to make the file as regular as possible, comments
stating the purpose of attributes have been removed. Instead,
links to the compiler docs have been added (i.e. to gcc and,
if available, to clang as well). In addition, they have been sorted.
Finally, if an attribute is optional (i.e. if it is guarded
by __has_attribute), the reason has been stated for future reference.
Cc: Rasmus Villemoes
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Nick Desaulniers
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Luc Van Oostenryck
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-clang.h | 4 -
include/linux/compiler-gcc.h| 51 --
include/linux/compiler-intel.h | 6 -
include/linux/compiler_attributes.h | 244
include/linux/compiler_types.h | 74 ++---
5 files changed, 254 insertions(+), 125 deletions(-)
create mode 100644 include/linux/compiler_attributes.h
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index fa9532f8d885..3e7dafb3ea80 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -21,8 +21,6 @@
#define __SANITIZE_ADDRESS__
#endif
-#define __no_sanitize_address __attribute__((__no_sanitize__("address")))
-
/*
* Not all versions of clang implement the the type-generic versions
* of the builtin overflow checkers. Fortunately, clang implements
@@ -41,5 +39,3 @@
* compilers, like ICC.
*/
#define barrier() __asm__ __volatile__("" : : : "memory")
-#define __assume_aligned(a, ...) \
- __attribute__((__assume_aligned__(a, ## __VA_ARGS__)))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 1ca6a51cfaa9..cfac027e1625 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -108,9 +108,6 @@
__builtin_unreachable();\
} while (0)
-/* Mark a function definition as prohibited from being cloned. */
-#define __noclone __attribute__((__noclone__, __optimize__("no-tracer")))
-
#if defined(RANDSTRUCT_PLUGIN) && !defined(__CHECKER__)
#define __randomize_layout __attribute__((randomize_layout))
#define __no_randomize_layout __attribute__((no_randomize_layout))
@@ -119,32 +116,6 @@
#define randomized_struct_fields_end } __randomize_layout;
#endif
-/*
- * When used with Link Time Optimization, gcc can optimize away C functions or
- * variables which are referenced only from assembly code. __visible tells the
- * optimizer that something else uses this function or variable, thus
preventing
- * this.
- */
-#define __visible __attribute__((__externally_visible__))
-
-/* gcc version specific checks */
-
-#if GCC_VERSION >= 40900
-/*
- * __assume_aligned(n, k): Tell the optimizer that the returned
- * pointer can be assumed to be k modulo n. The second argument is
- * optional (default 0), so we use a variadic macro to make the
- * shorthand.
- *
- * Beware: Do not apply this to functions which may return
- * ERR_PTRs. Also, it is probably unwise to apply it to functions
- * returning extra information in the low bits (but in that case the
- * compiler should see some alignment anyway, when the return value is
- * massaged by 'flags = ptr & 3; ptr &= ~3;').
- */
-#define __assume_aligned(a, ...) __attribute__((__assume_aligned__(a, ##
__VA_ARGS__)))
-#endif
-
/*
* GCC 'asm goto' miscompiles certain code sequences:
*
@@ -176,32 +147,10 @@
#define KASAN_ABI_VERSION 3
#endif
-#if GCC_VERSION >= 40902
-/*
- * Tell the compiler that address safety instrumentation (KASAN)
- * should not be applied to that function.
- * Conflicts with inlining: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
- */
-#define __no_sanitize_address __attribute__((__no_sanitize_address__))
-#endif
-
#if GCC_VERSION >= 50100
-/*
- * Mark structures as requiring designated initializers.
- * https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
- */
-#define __designated_init __attribute__((__designated_init__))
#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
#endif
-#if !defined(__noclone)
-#define __noclone /* not needed */
-#endif
-
-#if !defin
[PATCH v4 00/13] Compiler Attributes
The Compiler Attributes series is an effort to disentangle the include/linux/compiler*.h headers and bring them up to date. The main idea behind the series is to use feature checking macros (i.e. __has_attribute) instead of compiler version checks (e.g. GCC_VERSION), which are compiler-agnostic (so they can be shared, reducing the size of compiler-specific headers) and version-agnostic. Other related improvements have been performed in the headers as well, which on top of the use of __has_attribute it has amounted to a significant simplification of these headers (e.g. GCC_VERSION is now only guarding 4 non-attribute macros). This series should also help the efforts to support compiling the kernel with clang and icc. A fair amount of documentation and comments have also been added, clarified or removed; and the headers are now more readable, which should help kernel developers in general. The series was triggered due to the move to gcc >= 4.6. In turn, this series has also triggered Sparse to gain the ability to recognize __has_attribute on its own. You can also fetch it from: https://github.com/ojeda/linux/tree/compiler-attributes-v4 Enjoy! Cheers, Miguel Cc: Jonathan Corbet Cc: Rasmus Villemoes Cc: Luc Van Oostenryck Cc: Eli Friedman Cc: Christopher Li Cc: Kees Cook Cc: Ingo Molnar Cc: Geert Uytterhoeven Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Masahiro Yamada Cc: Joe Perches Cc: Dominique Martinet Cc: Nick Desaulniers Cc: Linus Torvalds Cc: linux-spa...@vger.kernel.org Cc: linux-...@vger.kernel.org Signed-off-by: Miguel Ojeda v3 -> v4 This time it is an important fix I found while doing randconfigs, plus the documentation sentence that Nick suggested and the MAINTAINERS file. Compile-tested for a while on (x86_64, gcc-7.3) on a few randconfs. New: * Add MAINTAINERS entry for the new file (compiler_attributes.h) so that we try to maintain that one as clean as possible. (if someone else wants to join, feel free!) Modified: * Fix inline macro __always_inline cannot be used in the inline macro, because people marking a function as __always_inline would expand to inline which in turn would expand back to __always_inline (under some configs), which would not be expanded again. Added a comment about this in the inline macro. Also added another comment about __always_inline in compiler_attributes.h to note that users expect to not have to write inline (as well as the attribute). We cannot simply remove "inline" there (which would solve the problem described above) because using the attribute is not enough for gcc. A function marked as always_inline seems to require to be marked as inline itself so that the attribute is applied: "warning: always_inline function might not be inlinable [-Wattributes]" From the gcc docs: "For functions declared inline, this attribute inlines the function independent of any restrictions that otherwise apply to inlining." See https://godbolt.org/z/LpzUPj for an example. From reviews: * Add sentence about feature-detection in Docs (Nick) Miguel Ojeda (13): Compiler Attributes: remove unused attributes Compiler Attributes: always use the extra-underscores syntax Compiler Attributes: remove unneeded tests Compiler Attributes: homogenize __must_be_array Compiler Attributes: naked was fixed in gcc 4.6 Compiler Attributes: naked can be shared Compiler Attributes: remove unneeded sparse (__CHECKER__) tests Compiler Attributes: add missing SPDX ID in compiler_types.h Compiler Attributes: use feature checks instead of version checks Compiler Attributes: KENTRY used twice the "used" attribute Compiler Attributes: remove uses of __attribute__ from compiler.h Compiler Attributes: add Doc/process/programming-language.rst Compiler Attributes: Add MAINTAINERS entry Documentation/process/index.rst | 1 + .../process/programming-language.rst | 45 MAINTAINERS | 5 + include/linux/compiler-clang.h| 5 - include/linux/compiler-gcc.h | 84 +- include/linux/compiler-intel.h| 9 - include/linux/compiler.h | 19 +- include/linux/compiler_attributes.h | 244 ++ include/linux/compiler_types.h| 105 ++-- 9 files changed, 329 insertions(+), 188 deletions(-) create mode 100644 Documentation/process/programming-language.rst create mode 100644 include/linux/compiler_attributes.h -- 2.17.1
[PATCH v4 08/13] Compiler Attributes: add missing SPDX ID in compiler_types.h
Cc: Rasmus Villemoes Cc: Luc Van Oostenryck Cc: Eli Friedman Cc: Christopher Li Cc: Kees Cook Cc: Ingo Molnar Cc: Geert Uytterhoeven Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Masahiro Yamada Cc: Joe Perches Cc: Dominique Martinet Cc: Nick Desaulniers Cc: Linus Torvalds Cc: linux-spa...@vger.kernel.org Signed-off-by: Miguel Ojeda --- include/linux/compiler_types.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index a3eceb3ad1b3..ed7c0e4a180e 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -1,3 +1,4 @@ +/* SPDX-License-Identifier: GPL-2.0 */ #ifndef __LINUX_COMPILER_TYPES_H #define __LINUX_COMPILER_TYPES_H -- 2.17.1
[PATCH v4 13/13] Compiler Attributes: Add MAINTAINERS entry
Cc: Eli Friedman Cc: Christopher Li Cc: Kees Cook Cc: Ingo Molnar Cc: Geert Uytterhoeven Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Masahiro Yamada Cc: Joe Perches Cc: Dominique Martinet Cc: Nick Desaulniers Cc: Linus Torvalds Signed-off-by: Miguel Ojeda --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9ad052aeac39..a7ef76d694d4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3721,6 +3721,11 @@ L: platform-driver-...@vger.kernel.org S: Maintained F: drivers/platform/x86/compal-laptop.c +COMPILER ATTRIBUTES +M: Miguel Ojeda +S: Maintained +F: include/linux/compiler_attributes.h + CONEXANT ACCESSRUNNER USB DRIVER L: accessrunner-gene...@lists.sourceforge.net W: http://accessrunner.sourceforge.net/ -- 2.17.1
[PATCH v4 02/13] Compiler Attributes: always use the extra-underscores syntax
The attribute syntax optionally allows to surround attribute names
with "__" in order to avoid collisions with macros of the same name
(see https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html).
This homogenizes all attributes to use the syntax with underscores.
While there are currently only a handful of cases of some TUs defining
macros like "error" which may collide with the attributes,
this should prevent futures surprises.
This has been done only for "standard" attributes supported by
the major compilers. In other words, those of third-party tools
(e.g. sparse, plugins...) have not been changed for the moment.
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-clang.h | 2 +-
include/linux/compiler-gcc.h | 14 ++--
include/linux/compiler-intel.h | 2 +-
include/linux/compiler.h | 8 +++
include/linux/compiler_types.h | 40 +-
5 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index b1ce500fe8b3..d11cad61ba5c 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -21,7 +21,7 @@
#define __SANITIZE_ADDRESS__
#endif
-#define __no_sanitize_address __attribute__((no_sanitize("address")))
+#define __no_sanitize_address __attribute__((__no_sanitize__("address")))
/*
* Not all versions of clang implement the the type-generic versions
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 0a2d06677d83..371b6fa2d074 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -76,7 +76,7 @@
#endif
#ifdef RETPOLINE
-#define __noretpoline __attribute__((indirect_branch("keep")))
+#define __noretpoline __attribute__((__indirect_branch__("keep")))
#endif
/*
@@ -91,15 +91,15 @@
* 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__)) noinline __noclone
notrace
#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
#define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
#ifndef __CHECKER__
-#define __compiletime_warning(message) __attribute__((warning(message)))
-#define __compiletime_error(message) __attribute__((error(message)))
+#define __compiletime_warning(message) __attribute__((__warning__(message)))
+#define __compiletime_error(message) __attribute__((__error__(message)))
#ifdef LATENT_ENTROPY_PLUGIN
#define __latent_entropy __attribute__((latent_entropy))
@@ -148,7 +148,7 @@
* optimizer that something else uses this function or variable, thus
preventing
* this.
*/
-#define __visible __attribute__((externally_visible))
+#define __visible __attribute__((__externally_visible__))
/* gcc version specific checks */
@@ -205,7 +205,7 @@
* should not be applied to that function.
* Conflicts with inlining: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
*/
-#define __no_sanitize_address __attribute__((no_sanitize_address))
+#define __no_sanitize_address __attribute__((__no_sanitize_address__))
#endif
#if GCC_VERSION >= 50100
@@ -213,7 +213,7 @@
* Mark structures as requiring designated initializers.
* https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
*/
-#define __designated_init __attribute__((designated_init))
+#define __designated_init __attribute__((__designated_init__))
#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
#endif
diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index 4c7f9befa9f6..fef8bb3e53ef 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -42,4 +42,4 @@
* and may be redefined here because they should not be shared with other
* compilers, like clang.
*/
-#define __visible __attribute__((externally_visible))
+#define __visible __attribute__((__externally_visible__))
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 7c0157d50964..ec4a28bad2c6 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -24,7 +24,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
long __r; \
static struct ftrace_likely_data\
__attribute__((__aligned__(4))) \
-
__attribute__((section("_ftrace_anno
[PATCH v4 01/13] Compiler Attributes: remove unused attributes
__optimize and __deprecate_for_modules are unused in
the whole kernel tree. Simply drop them.
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-gcc.h | 2 --
include/linux/compiler.h | 4
include/linux/compiler_types.h | 1 -
3 files changed, 7 deletions(-)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 763bbad1e258..0a2d06677d83 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -95,8 +95,6 @@
#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
-#define __optimize(level) __attribute__((__optimize__(level)))
-
#define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
#ifndef __CHECKER__
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 681d866efb1e..7c0157d50964 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -301,10 +301,6 @@ static inline void *offset_to_ptr(const int *off)
#endif /* __ASSEMBLY__ */
-#ifndef __optimize
-# define __optimize(level)
-#endif
-
/* Compile time object size, -1 for unknown */
#ifndef __compiletime_object_size
# define __compiletime_object_size(obj) -1
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 3525c179698c..b6534292ea33 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -108,7 +108,6 @@ struct ftrace_likely_data {
/* Don't. Just don't. */
#define __deprecated
-#define __deprecated_for_modules
#endif /* __KERNEL__ */
--
2.17.1
[PATCH v4 12/13] Compiler Attributes: add Doc/process/programming-language.rst
Cc: Jonathan Corbet Cc: Rasmus Villemoes Cc: Luc Van Oostenryck Cc: Eli Friedman Cc: Christopher Li Cc: Kees Cook Cc: Ingo Molnar Cc: Geert Uytterhoeven Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Masahiro Yamada Cc: Joe Perches Cc: Dominique Martinet Cc: Nick Desaulniers Cc: Linus Torvalds Cc: linux-spa...@vger.kernel.org Cc: linux-...@vger.kernel.org Signed-off-by: Miguel Ojeda --- Documentation/process/index.rst | 1 + .../process/programming-language.rst | 45 +++ 2 files changed, 46 insertions(+) create mode 100644 Documentation/process/programming-language.rst diff --git a/Documentation/process/index.rst b/Documentation/process/index.rst index 37bd0628b6ee..c56f24a22d2a 100644 --- a/Documentation/process/index.rst +++ b/Documentation/process/index.rst @@ -23,6 +23,7 @@ Below are the essential guides that every developer should read. code-of-conflict development-process submitting-patches + programming-language coding-style maintainer-pgp-guide email-clients diff --git a/Documentation/process/programming-language.rst b/Documentation/process/programming-language.rst new file mode 100644 index ..e5f5f065dc24 --- /dev/null +++ b/Documentation/process/programming-language.rst @@ -0,0 +1,45 @@ +.. _programming_language: + +Programming Language + + +The kernel is written in the C programming language [c-language]_. +More precisely, the kernel is typically compiled with ``gcc`` [gcc]_ +under ``-std=gnu89`` [gcc-c-dialect-options]_: the GNU dialect of ISO C90 +(including some C99 features). + +This dialect contains many extensions to the language [gnu-extensions]_, +and many of them are used within the kernel as a matter of course. + +There is some support for compiling the kernel with ``clang`` [clang]_ +and ``icc`` [icc]_ for several of the architectures, although at the time +of writing it is not completed, requiring third-party patches. + +Attributes +-- + +One of the common extensions used throughout the kernel are attributes +[gcc-attribute-syntax]_. Attributes allow to introduce +implementation-defined semantics to language entities (like variables, +functions or types) without having to make significant syntactic changes +to the language (e.g. adding a new keyword) [n2049]_. + +In some cases, attributes are optional (i.e. a compiler not supporting them +should still produce proper code, even if it is slower or does not perform +as many compile-time checks/diagnostics). + +The kernel defines pseudo-keywords (e.g. ``__pure``) instead of using +directly the GNU attribute syntax (e.g. ``__attribute__((__pure__))``) +in order to feature detect which ones can be used and/or to shorten the code. + +Please refer to ``include/linux/compiler_attributes.h`` for more information. + +.. [c-language] http://www.open-std.org/jtc1/sc22/wg14/www/standards +.. [gcc] https://gcc.gnu.org +.. [clang] https://clang.llvm.org +.. [icc] https://software.intel.com/en-us/c-compilers +.. [gcc-c-dialect-options] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html +.. [gnu-extensions] https://gcc.gnu.org/onlinedocs/gcc/C-Extensions.html +.. [gcc-attribute-syntax] https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html +.. [n2049] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2049.pdf + -- 2.17.1
[PATCH v4 11/13] Compiler Attributes: remove uses of __attribute__ from compiler.h
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Suggested-by: Nick Desaulniers
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 17ee9165ca51..b5fb034fa6fa 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -23,8 +23,8 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
#define __branch_check__(x, expect, is_constant) ({\
long __r; \
static struct ftrace_likely_data\
- __attribute__((__aligned__(4))) \
-
__attribute__((__section__("_ftrace_annotated_branch"))) \
+ __aligned(4)\
+ __section("_ftrace_annotated_branch") \
__f = { \
.data.func = __func__, \
.data.file = __FILE__, \
@@ -59,8 +59,8 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
({ \
int __r;\
static struct ftrace_branch_data\
- __attribute__((__aligned__(4))) \
- __attribute__((__section__("_ftrace_branch"))) \
+ __aligned(4)\
+ __section("_ftrace_branch") \
__f = { \
.func = __func__, \
.file = __FILE__, \
@@ -146,7 +146,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
extern typeof(sym) sym; \
static const unsigned long __kentry_##sym \
__used \
- __attribute__((__section__("___kentry" "+" #sym ))) \
+ __section("___kentry" "+" #sym )\
= (unsigned long)
#endif
@@ -287,7 +287,7 @@ unsigned long read_word_at_a_time(const void *addr)
* visible to the compiler.
*/
#define __ADDRESSABLE(sym) \
- static void * __attribute__((__section__(".discard.addressable"),
used)) \
+ static void * __section(".discard.addressable") __used \
__PASTE(__addressable_##sym, __LINE__) = (void *)
/**
--
2.17.1
[PATCH v4 06/13] Compiler Attributes: naked can be shared
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.
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Nick Desaulniers
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Suggested-by: Arnd Bergmann
Signed-off-by: Miguel Ojeda
---
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 4cd5e9264bce..3b32bbfa5a49 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -72,14 +72,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 __compiletime_object_size(obj) __builtin_object_size(obj, 0)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 83475515bc39..5ff9cda893f4 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -224,6 +224,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
[PATCH v4 11/13] Compiler Attributes: remove uses of __attribute__ from compiler.h
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Suggested-by: Nick Desaulniers
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 17ee9165ca51..b5fb034fa6fa 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -23,8 +23,8 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
#define __branch_check__(x, expect, is_constant) ({\
long __r; \
static struct ftrace_likely_data\
- __attribute__((__aligned__(4))) \
-
__attribute__((__section__("_ftrace_annotated_branch"))) \
+ __aligned(4)\
+ __section("_ftrace_annotated_branch") \
__f = { \
.data.func = __func__, \
.data.file = __FILE__, \
@@ -59,8 +59,8 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
({ \
int __r;\
static struct ftrace_branch_data\
- __attribute__((__aligned__(4))) \
- __attribute__((__section__("_ftrace_branch"))) \
+ __aligned(4)\
+ __section("_ftrace_branch") \
__f = { \
.func = __func__, \
.file = __FILE__, \
@@ -146,7 +146,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
extern typeof(sym) sym; \
static const unsigned long __kentry_##sym \
__used \
- __attribute__((__section__("___kentry" "+" #sym ))) \
+ __section("___kentry" "+" #sym )\
= (unsigned long)
#endif
@@ -287,7 +287,7 @@ unsigned long read_word_at_a_time(const void *addr)
* visible to the compiler.
*/
#define __ADDRESSABLE(sym) \
- static void * __attribute__((__section__(".discard.addressable"),
used)) \
+ static void * __section(".discard.addressable") __used \
__PASTE(__addressable_##sym, __LINE__) = (void *)
/**
--
2.17.1
[PATCH v4 06/13] Compiler Attributes: naked can be shared
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.
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Nick Desaulniers
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Suggested-by: Arnd Bergmann
Signed-off-by: Miguel Ojeda
---
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 4cd5e9264bce..3b32bbfa5a49 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -72,14 +72,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 __compiletime_object_size(obj) __builtin_object_size(obj, 0)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 83475515bc39..5ff9cda893f4 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -224,6 +224,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
[PATCH] kernel/sched/core.c: Avoid unused variable on non-SMP configs
On non-SMP configs, when only one of CONFIG_{PARAVIRT,IRQ_TIME}_ACCOUNTING
is defined, we are declaring a variable (irq_delta or steal) which
is not used:
kernel/sched/core.c: In function ‘update_rq_clock_task’:
kernel/sched/core.c:139:17: warning: unused variable ‘irq_delta’
[-Wunused-variable]
s64 steal = 0, irq_delta = 0;
The reason is that CONFIG_SMP guards HAVE_SCHED_AVG_IRQ, which in turn
disables the code guarded by HAVE_SCHED_AVG_IRQ.
Cc: Ingo Molnar
Cc: Peter Zijlstra
Signed-off-by: Miguel Ojeda
---
kernel/sched/core.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 625bc9897f62..d662d1e11843 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -135,8 +135,11 @@ static void update_rq_clock_task(struct rq *rq, s64 delta)
* In theory, the compile should just see 0 here, and optimize out the call
* to sched_rt_avg_update. But I don't trust it...
*/
-#if defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
- s64 steal = 0, irq_delta = 0;
+#if defined(HAVE_SCHED_AVG_IRQ) || defined(CONFIG_IRQ_TIME_ACCOUNTING)
+ s64 irq_delta = 0;
+#endif
+#if defined(HAVE_SCHED_AVG_IRQ) || defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
+ s64 steal = 0;
#endif
#ifdef CONFIG_IRQ_TIME_ACCOUNTING
irq_delta = irq_time_read(cpu_of(rq)) - rq->prev_irq_time;
--
2.17.1
[PATCH] kernel/sched/core.c: Avoid unused variable on non-SMP configs
On non-SMP configs, when only one of CONFIG_{PARAVIRT,IRQ_TIME}_ACCOUNTING
is defined, we are declaring a variable (irq_delta or steal) which
is not used:
kernel/sched/core.c: In function ‘update_rq_clock_task’:
kernel/sched/core.c:139:17: warning: unused variable ‘irq_delta’
[-Wunused-variable]
s64 steal = 0, irq_delta = 0;
The reason is that CONFIG_SMP guards HAVE_SCHED_AVG_IRQ, which in turn
disables the code guarded by HAVE_SCHED_AVG_IRQ.
Cc: Ingo Molnar
Cc: Peter Zijlstra
Signed-off-by: Miguel Ojeda
---
kernel/sched/core.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 625bc9897f62..d662d1e11843 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -135,8 +135,11 @@ static void update_rq_clock_task(struct rq *rq, s64 delta)
* In theory, the compile should just see 0 here, and optimize out the call
* to sched_rt_avg_update. But I don't trust it...
*/
-#if defined(CONFIG_IRQ_TIME_ACCOUNTING) ||
defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
- s64 steal = 0, irq_delta = 0;
+#if defined(HAVE_SCHED_AVG_IRQ) || defined(CONFIG_IRQ_TIME_ACCOUNTING)
+ s64 irq_delta = 0;
+#endif
+#if defined(HAVE_SCHED_AVG_IRQ) || defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
+ s64 steal = 0;
#endif
#ifdef CONFIG_IRQ_TIME_ACCOUNTING
irq_delta = irq_time_read(cpu_of(rq)) - rq->prev_irq_time;
--
2.17.1
Re: [PATCH v13 11/13] platform/x86: Intel SGX driver
Hi Jarkko, On Thu, Sep 6, 2018 at 11:21 AM, Jarkko Sakkinen wrote: > There is another open. If I grep through the kernel tree I see SPDX > headers that are decorated both with C99- and C89-style comments. I > guess I ended up using C99-style because when I was instructed to add > SPDX headers in the first place that was the example I was given. Still > checkpatch.pl complains about C99-style comments. > > Which one is right and why the kernel tree is polluted with C99-headers > when they do not pass checkpatch.pl? How those commits were ever > accepted? See Documentation/process/license-rules.rst. Headers should go with C-style comments: The SPDX license identifier is added in form of a comment. The comment style depends on the file type:: C source: // SPDX-License-Identifier: C header: /* SPDX-License-Identifier: */ And: If a specific tool cannot handle the standard comment style, then the appropriate comment mechanism which the tool accepts shall be used. This is the reason for having the "/\* \*/" style comment in C header files. There was build breakage observed with generated .lds files where 'ld' failed to parse the C++ comment. This has been fixed by now, but there are still older assembler tools which cannot handle C++ style comments. The ones that got in are probably either old or they slipped through (and they do not break the build). Cheers, Miguel
Re: [PATCH v13 11/13] platform/x86: Intel SGX driver
Hi Jarkko, On Thu, Sep 6, 2018 at 11:21 AM, Jarkko Sakkinen wrote: > There is another open. If I grep through the kernel tree I see SPDX > headers that are decorated both with C99- and C89-style comments. I > guess I ended up using C99-style because when I was instructed to add > SPDX headers in the first place that was the example I was given. Still > checkpatch.pl complains about C99-style comments. > > Which one is right and why the kernel tree is polluted with C99-headers > when they do not pass checkpatch.pl? How those commits were ever > accepted? See Documentation/process/license-rules.rst. Headers should go with C-style comments: The SPDX license identifier is added in form of a comment. The comment style depends on the file type:: C source: // SPDX-License-Identifier: C header: /* SPDX-License-Identifier: */ And: If a specific tool cannot handle the standard comment style, then the appropriate comment mechanism which the tool accepts shall be used. This is the reason for having the "/\* \*/" style comment in C header files. There was build breakage observed with generated .lds files where 'ld' failed to parse the C++ comment. This has been fixed by now, but there are still older assembler tools which cannot handle C++ style comments. The ones that got in are probably either old or they slipped through (and they do not break the build). Cheers, Miguel
[PATCH 04/12] Compiler Attributes: homogenize __must_be_array
Different definitions of __must_be_array:
* gcc: disabled for __CHECKER__
* clang: same definition as gcc's, but without __CHECKER__
* intel: the comment claims __builtin_types_compatible_p()
is unsupported; but icc seems to support it since 13.0.1
(released in 2012). See https://godbolt.org/z/S0l6QQ
Therefore, we can remove all of them and have a single definition
in compiler.h
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-clang.h | 1 -
include/linux/compiler-gcc.h | 7 ---
include/linux/compiler-intel.h | 3 ---
include/linux/compiler.h | 7 +++
4 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index d11cad61ba5c..fa9532f8d885 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -41,6 +41,5 @@
* compilers, like ICC.
*/
#define barrier() __asm__ __volatile__("" : : : "memory")
-#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
#define __assume_aligned(a, ...) \
__attribute__((__assume_aligned__(a, ## __VA_ARGS__)))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 371b6fa2d074..76f4907ef707 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -68,13 +68,6 @@
*/
#define uninitialized_var(x) x = x
-#ifdef __CHECKER__
-#define __must_be_array(a) 0
-#else
-/* [0] degrades to a pointer: a different type from an array */
-#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
-#endif
-
#ifdef RETPOLINE
#define __noretpoline __attribute__((__indirect_branch__("keep")))
#endif
diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index fef8bb3e53ef..6004b4588bd4 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -29,9 +29,6 @@
*/
#define OPTIMIZER_HIDE_VAR(var) barrier()
-/* Intel ECC compiler doesn't support __builtin_types_compatible_p() */
-#define __must_be_array(a) 0
-
#endif
/* icc has this, but it's called _bswap16 */
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index ec4a28bad2c6..165b1d5683ed 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -357,4 +357,11 @@ static inline void *offset_to_ptr(const int *off)
compiletime_assert(__native_word(t),\
"Need native word sized stores/loads for atomicity.")
+#ifdef __CHECKER__
+#define __must_be_array(a) 0
+#else
+/* [0] degrades to a pointer: a different type from an array */
+#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
+#endif
+
#endif /* __LINUX_COMPILER_H */
--
2.17.1
[PATCH 04/12] Compiler Attributes: homogenize __must_be_array
Different definitions of __must_be_array:
* gcc: disabled for __CHECKER__
* clang: same definition as gcc's, but without __CHECKER__
* intel: the comment claims __builtin_types_compatible_p()
is unsupported; but icc seems to support it since 13.0.1
(released in 2012). See https://godbolt.org/z/S0l6QQ
Therefore, we can remove all of them and have a single definition
in compiler.h
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler-clang.h | 1 -
include/linux/compiler-gcc.h | 7 ---
include/linux/compiler-intel.h | 3 ---
include/linux/compiler.h | 7 +++
4 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index d11cad61ba5c..fa9532f8d885 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -41,6 +41,5 @@
* compilers, like ICC.
*/
#define barrier() __asm__ __volatile__("" : : : "memory")
-#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
#define __assume_aligned(a, ...) \
__attribute__((__assume_aligned__(a, ## __VA_ARGS__)))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 371b6fa2d074..76f4907ef707 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -68,13 +68,6 @@
*/
#define uninitialized_var(x) x = x
-#ifdef __CHECKER__
-#define __must_be_array(a) 0
-#else
-/* [0] degrades to a pointer: a different type from an array */
-#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
-#endif
-
#ifdef RETPOLINE
#define __noretpoline __attribute__((__indirect_branch__("keep")))
#endif
diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index fef8bb3e53ef..6004b4588bd4 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -29,9 +29,6 @@
*/
#define OPTIMIZER_HIDE_VAR(var) barrier()
-/* Intel ECC compiler doesn't support __builtin_types_compatible_p() */
-#define __must_be_array(a) 0
-
#endif
/* icc has this, but it's called _bswap16 */
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index ec4a28bad2c6..165b1d5683ed 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -357,4 +357,11 @@ static inline void *offset_to_ptr(const int *off)
compiletime_assert(__native_word(t),\
"Need native word sized stores/loads for atomicity.")
+#ifdef __CHECKER__
+#define __must_be_array(a) 0
+#else
+/* [0] degrades to a pointer: a different type from an array */
+#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
+#endif
+
#endif /* __LINUX_COMPILER_H */
--
2.17.1
[PATCH 03/12] Compiler Attributes: remove unneeded tests
Attributes const and always_inline have tests around them
which are unneeded, since they are supported by gcc >= 4.6,
clang >= 3 and icc >= 13. https://godbolt.org/z/DFPq37
In the case of gnu_inline, we do not need to test for
__GNUC_STDC_INLINE__ because, regardless of the current
inlining behavior, we can simply always force the old
GCC inlining behavior by using the attribute in all cases.
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler_types.h | 24 +++-
1 file changed, 3 insertions(+), 21 deletions(-)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 2bc0f94df38e..759b3fea9216 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -158,10 +158,6 @@ struct ftrace_likely_data {
(sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || \
sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
-#ifndef __attribute_const__
-#define __attribute_const____attribute__((__const__))
-#endif
-
#ifndef __noclone
#define __noclone
#endif
@@ -196,6 +192,7 @@ struct ftrace_likely_data {
* [...]
*/
#define __pure __attribute__((__pure__))
+#define __attribute_const____attribute__((__const__))
#define __aligned(x) __attribute__((__aligned__(x)))
#define __aligned_largest __attribute__((__aligned__))
#define __printf(a, b) __attribute__((__format__(printf, a, b)))
@@ -211,7 +208,8 @@ struct ftrace_likely_data {
#define __alias(symbol)__attribute__((__alias__(#symbol)))
#define __cold __attribute__((__cold__))
#define __section(S) __attribute__((__section__(#S)))
-
+#define __gnu_inline __attribute__((__gnu_inline__))
+#define __always_inlineinline
__attribute__((__always_inline__))
#ifdef CONFIG_ENABLE_MUST_CHECK
#define __must_check __attribute__((__warn_unused_result__))
@@ -227,18 +225,6 @@ struct ftrace_likely_data {
#define __compiler_offsetof(a, b) __builtin_offsetof(a, b)
-/*
- * Feature detection for gnu_inline (gnu89 extern inline semantics). Either
- * __GNUC_STDC_INLINE__ is defined (not using gnu89 extern inline semantics,
- * and we opt in to the gnu89 semantics), or __GNUC_STDC_INLINE__ is not
- * defined so the gnu89 semantics are the default.
- */
-#ifdef __GNUC_STDC_INLINE__
-# define __gnu_inline __attribute__((__gnu_inline__))
-#else
-# define __gnu_inline
-#endif
-
/*
* Force always-inline if the user requests it so via the .config.
* GCC does not warn about unused static inline functions for
@@ -263,10 +249,6 @@ struct ftrace_likely_data {
#define __inline inline
#define noinline __attribute__((__noinline__))
-#ifndef __always_inline
-#define __always_inline inline __attribute__((__always_inline__))
-#endif
-
/*
* Rather then using noinline to prevent stack consumption, use
* noinline_for_stack instead. For documentation reasons.
--
2.17.1
[PATCH 03/12] Compiler Attributes: remove unneeded tests
Attributes const and always_inline have tests around them
which are unneeded, since they are supported by gcc >= 4.6,
clang >= 3 and icc >= 13. https://godbolt.org/z/DFPq37
In the case of gnu_inline, we do not need to test for
__GNUC_STDC_INLINE__ because, regardless of the current
inlining behavior, we can simply always force the old
GCC inlining behavior by using the attribute in all cases.
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler_types.h | 24 +++-
1 file changed, 3 insertions(+), 21 deletions(-)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 2bc0f94df38e..759b3fea9216 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -158,10 +158,6 @@ struct ftrace_likely_data {
(sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || \
sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
-#ifndef __attribute_const__
-#define __attribute_const____attribute__((__const__))
-#endif
-
#ifndef __noclone
#define __noclone
#endif
@@ -196,6 +192,7 @@ struct ftrace_likely_data {
* [...]
*/
#define __pure __attribute__((__pure__))
+#define __attribute_const____attribute__((__const__))
#define __aligned(x) __attribute__((__aligned__(x)))
#define __aligned_largest __attribute__((__aligned__))
#define __printf(a, b) __attribute__((__format__(printf, a, b)))
@@ -211,7 +208,8 @@ struct ftrace_likely_data {
#define __alias(symbol)__attribute__((__alias__(#symbol)))
#define __cold __attribute__((__cold__))
#define __section(S) __attribute__((__section__(#S)))
-
+#define __gnu_inline __attribute__((__gnu_inline__))
+#define __always_inlineinline
__attribute__((__always_inline__))
#ifdef CONFIG_ENABLE_MUST_CHECK
#define __must_check __attribute__((__warn_unused_result__))
@@ -227,18 +225,6 @@ struct ftrace_likely_data {
#define __compiler_offsetof(a, b) __builtin_offsetof(a, b)
-/*
- * Feature detection for gnu_inline (gnu89 extern inline semantics). Either
- * __GNUC_STDC_INLINE__ is defined (not using gnu89 extern inline semantics,
- * and we opt in to the gnu89 semantics), or __GNUC_STDC_INLINE__ is not
- * defined so the gnu89 semantics are the default.
- */
-#ifdef __GNUC_STDC_INLINE__
-# define __gnu_inline __attribute__((__gnu_inline__))
-#else
-# define __gnu_inline
-#endif
-
/*
* Force always-inline if the user requests it so via the .config.
* GCC does not warn about unused static inline functions for
@@ -263,10 +249,6 @@ struct ftrace_likely_data {
#define __inline inline
#define noinline __attribute__((__noinline__))
-#ifndef __always_inline
-#define __always_inline inline __attribute__((__always_inline__))
-#endif
-
/*
* Rather then using noinline to prevent stack consumption, use
* noinline_for_stack instead. For documentation reasons.
--
2.17.1
[PATCH 11/12] Compiler Attributes: remove uses of __attribute__ from compiler.h
Cc: Rasmus Villemoes
Cc: Luc Van Oostenryck
Cc: Eli Friedman
Cc: Christopher Li
Cc: Kees Cook
Cc: Ingo Molnar
Cc: Geert Uytterhoeven
Cc: Arnd Bergmann
Cc: Greg Kroah-Hartman
Cc: Masahiro Yamada
Cc: Joe Perches
Cc: Dominique Martinet
Cc: Linus Torvalds
Cc: linux-spa...@vger.kernel.org
Suggested-by: Nick Desaulniers
Reviewed-by: Nick Desaulniers
Signed-off-by: Miguel Ojeda
---
include/linux/compiler.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 17ee9165ca51..b5fb034fa6fa 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -23,8 +23,8 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
#define __branch_check__(x, expect, is_constant) ({\
long __r; \
static struct ftrace_likely_data\
- __attribute__((__aligned__(4))) \
-
__attribute__((__section__("_ftrace_annotated_branch"))) \
+ __aligned(4)\
+ __section("_ftrace_annotated_branch") \
__f = { \
.data.func = __func__, \
.data.file = __FILE__, \
@@ -59,8 +59,8 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
({ \
int __r;\
static struct ftrace_branch_data\
- __attribute__((__aligned__(4))) \
- __attribute__((__section__("_ftrace_branch"))) \
+ __aligned(4)\
+ __section("_ftrace_branch") \
__f = { \
.func = __func__, \
.file = __FILE__, \
@@ -146,7 +146,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int
val,
extern typeof(sym) sym; \
static const unsigned long __kentry_##sym \
__used \
- __attribute__((__section__("___kentry" "+" #sym ))) \
+ __section("___kentry" "+" #sym )\
= (unsigned long)
#endif
@@ -287,7 +287,7 @@ unsigned long read_word_at_a_time(const void *addr)
* visible to the compiler.
*/
#define __ADDRESSABLE(sym) \
- static void * __attribute__((__section__(".discard.addressable"),
used)) \
+ static void * __section(".discard.addressable") __used \
__PASTE(__addressable_##sym, __LINE__) = (void *)
/**
--
2.17.1
