2022-09-27 22:07:30

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH] include/uapi/linux/swab: Fix potentially missing __always_inline

From: Matt Redfearn <[email protected]>

Commit bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining
of some byteswap operations") added __always_inline to swab functions
and commit 283d75737837 ("uapi/linux/stddef.h: Provide __always_inline to
userspace headers") added a definition of __always_inline for use in
exported headers when the kernel's compiler.h is not available.

However, since swab.h does not include stddef.h, if the header soup does
not indirectly include it, the definition of __always_inline is missing,
resulting in a compilation failure, which was observed compiling the
perf tool using exported headers containing this commit:

In file included from /usr/include/linux/byteorder/little_endian.h:12:0,
from /usr/include/asm/byteorder.h:14,
from tools/include/uapi/linux/perf_event.h:20,
from perf.h:8,
from builtin-bench.c:18:
/usr/include/linux/swab.h:160:8: error: unknown type name `__always_inline'
static __always_inline __u16 __swab16p(const __u16 *p)

Fix this by replacing the inclusion of linux/compiler.h with
linux/stddef.h to ensure that we pick up that definition if required,
without relying on it's indirect inclusion. compiler.h is then included
indirectly, via stddef.h.

Fixes: 283d75737837 ("uapi/linux/stddef.h: Provide __always_inline to userspace headers")
Signed-off-by: Matt Redfearn <[email protected]>
Reviewed-by: Petr Vaněk <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
Apologies for the insanely long CC list. I encountered this problem
again while attempting to build perf with a LLVM 13 toolchain using
musl-libc. Not sure why it did not get picked up last time?

Khem did submit an alternative patch a few years ago, too which also did
not get picked up:

https://lore.kernel.org/lkml/[email protected]/

include/uapi/linux/swab.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h
index 0723a9cce747..01717181339e 100644
--- a/include/uapi/linux/swab.h
+++ b/include/uapi/linux/swab.h
@@ -3,7 +3,7 @@
#define _UAPI_LINUX_SWAB_H

#include <linux/types.h>
-#include <linux/compiler.h>
+#include <linux/stddef.h>
#include <asm/bitsperlong.h>
#include <asm/swab.h>

--
2.25.1


2022-09-28 23:17:02

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] include/uapi/linux/swab: Fix potentially missing __always_inline

Hi Florian,

On Tue, Sep 27, 2022 at 02:52:56PM -0700, Florian Fainelli wrote:
> From: Matt Redfearn <[email protected]>
>
> Commit bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining
> of some byteswap operations") added __always_inline to swab functions
> and commit 283d75737837 ("uapi/linux/stddef.h: Provide __always_inline to
> userspace headers") added a definition of __always_inline for use in
> exported headers when the kernel's compiler.h is not available.
>
> However, since swab.h does not include stddef.h, if the header soup does
> not indirectly include it, the definition of __always_inline is missing,
> resulting in a compilation failure, which was observed compiling the
> perf tool using exported headers containing this commit:
>
> In file included from /usr/include/linux/byteorder/little_endian.h:12:0,
> from /usr/include/asm/byteorder.h:14,
> from tools/include/uapi/linux/perf_event.h:20,
> from perf.h:8,
> from builtin-bench.c:18:
> /usr/include/linux/swab.h:160:8: error: unknown type name `__always_inline'
> static __always_inline __u16 __swab16p(const __u16 *p)
>
> Fix this by replacing the inclusion of linux/compiler.h with
> linux/stddef.h to ensure that we pick up that definition if required,
> without relying on it's indirect inclusion. compiler.h is then included
> indirectly, via stddef.h.
>
> Fixes: 283d75737837 ("uapi/linux/stddef.h: Provide __always_inline to userspace headers")
> Signed-off-by: Matt Redfearn <[email protected]>
> Reviewed-by: Petr Vaněk <[email protected]>
> Signed-off-by: Florian Fainelli <[email protected]>

I took this through my kernel build matrix and did not see any new
issues.

Tested-by: Nathan Chancellor <[email protected]>

Cheers,
Nathan

> ---
> Apologies for the insanely long CC list. I encountered this problem
> again while attempting to build perf with a LLVM 13 toolchain using
> musl-libc. Not sure why it did not get picked up last time?
>
> Khem did submit an alternative patch a few years ago, too which also did
> not get picked up:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> include/uapi/linux/swab.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h
> index 0723a9cce747..01717181339e 100644
> --- a/include/uapi/linux/swab.h
> +++ b/include/uapi/linux/swab.h
> @@ -3,7 +3,7 @@
> #define _UAPI_LINUX_SWAB_H
>
> #include <linux/types.h>
> -#include <linux/compiler.h>
> +#include <linux/stddef.h>
> #include <asm/bitsperlong.h>
> #include <asm/swab.h>
>
> --
> 2.25.1
>
>

2022-10-06 20:41:01

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH] include/uapi/linux/swab: Fix potentially missing __always_inline

Hi Nathan,

On 9/28/2022 3:36 PM, Nathan Chancellor wrote:
> Hi Florian,
>
> On Tue, Sep 27, 2022 at 02:52:56PM -0700, Florian Fainelli wrote:
>> From: Matt Redfearn <[email protected]>
>>
>> Commit bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining
>> of some byteswap operations") added __always_inline to swab functions
>> and commit 283d75737837 ("uapi/linux/stddef.h: Provide __always_inline to
>> userspace headers") added a definition of __always_inline for use in
>> exported headers when the kernel's compiler.h is not available.
>>
>> However, since swab.h does not include stddef.h, if the header soup does
>> not indirectly include it, the definition of __always_inline is missing,
>> resulting in a compilation failure, which was observed compiling the
>> perf tool using exported headers containing this commit:
>>
>> In file included from /usr/include/linux/byteorder/little_endian.h:12:0,
>> from /usr/include/asm/byteorder.h:14,
>> from tools/include/uapi/linux/perf_event.h:20,
>> from perf.h:8,
>> from builtin-bench.c:18:
>> /usr/include/linux/swab.h:160:8: error: unknown type name `__always_inline'
>> static __always_inline __u16 __swab16p(const __u16 *p)
>>
>> Fix this by replacing the inclusion of linux/compiler.h with
>> linux/stddef.h to ensure that we pick up that definition if required,
>> without relying on it's indirect inclusion. compiler.h is then included
>> indirectly, via stddef.h.
>>
>> Fixes: 283d75737837 ("uapi/linux/stddef.h: Provide __always_inline to userspace headers")
>> Signed-off-by: Matt Redfearn <[email protected]>
>> Reviewed-by: Petr Vaněk <[email protected]>
>> Signed-off-by: Florian Fainelli <[email protected]>
>
> I took this through my kernel build matrix and did not see any new
> issues.
>
> Tested-by: Nathan Chancellor <[email protected]>

Great thanks! Since this is really useful for kernel-headers in
toolchains, it would be great to get this picked up. Arnd is this
something that would go via your tree?
--
Florian

2022-10-28 19:25:37

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH] include/uapi/linux/swab: Fix potentially missing __always_inline

On 10/6/22 13:33, Florian Fainelli wrote:
> Hi Nathan,
>
> On 9/28/2022 3:36 PM, Nathan Chancellor wrote:
>> Hi Florian,
>>
>> On Tue, Sep 27, 2022 at 02:52:56PM -0700, Florian Fainelli wrote:
>>> From: Matt Redfearn <[email protected]>
>>>
>>> Commit bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining
>>> of some byteswap operations") added __always_inline to swab functions
>>> and commit 283d75737837 ("uapi/linux/stddef.h: Provide
>>> __always_inline to
>>> userspace headers") added a definition of __always_inline for use in
>>> exported headers when the kernel's compiler.h is not available.
>>>
>>> However, since swab.h does not include stddef.h, if the header soup does
>>> not indirectly include it, the definition of __always_inline is missing,
>>> resulting in a compilation failure, which was observed compiling the
>>> perf tool using exported headers containing this commit:
>>>
>>> In file included from /usr/include/linux/byteorder/little_endian.h:12:0,
>>>                   from /usr/include/asm/byteorder.h:14,
>>>                   from tools/include/uapi/linux/perf_event.h:20,
>>>                   from perf.h:8,
>>>                   from builtin-bench.c:18:
>>> /usr/include/linux/swab.h:160:8: error: unknown type name
>>> `__always_inline'
>>>   static __always_inline __u16 __swab16p(const __u16 *p)
>>>
>>> Fix this by replacing the inclusion of linux/compiler.h with
>>> linux/stddef.h to ensure that we pick up that definition if required,
>>> without relying on it's indirect inclusion. compiler.h is then included
>>> indirectly, via stddef.h.
>>>
>>> Fixes: 283d75737837 ("uapi/linux/stddef.h: Provide __always_inline to
>>> userspace headers")
>>> Signed-off-by: Matt Redfearn <[email protected]>
>>> Reviewed-by: Petr Vaněk <[email protected]>
>>> Signed-off-by: Florian Fainelli <[email protected]>
>>
>> I took this through my kernel build matrix and did not see any new
>> issues.
>>
>> Tested-by: Nathan Chancellor <[email protected]>
>
> Great thanks! Since this is really useful for kernel-headers in
> toolchains, it would be great to get this picked up. Arnd is this
> something that would go via your tree?

Arnd, is this a patch you can merge? Thanks
--
Florian