Patchwork riscv: Use __has_include__ to include <asm/syscalls.h> [BZ #24022]

login
register
mail settings
Submitter H.J. Lu
Date Dec. 21, 2018, 5:48 p.m.
Message ID <20181221174840.10933-1-hjl.tools@gmail.com>
Download mbox | patch
Permalink /patch/30795/
State New
Headers show

Comments

H.J. Lu - Dec. 21, 2018, 5:48 p.m.
<asm/syscalls.h> has been removed by

commit 27f8899d6002e11a6e2d995e29b8deab5aa9cc25
Author: David Abdurachmanov <david.abdurachmanov@gmail.com>
Date:   Thu Nov 8 20:02:39 2018 +0100

    riscv: add asm/unistd.h UAPI header

    Marcin Juszkiewicz reported issues while generating syscall table for riscv
    using 4.20-rc1. The patch refactors our unistd.h files to match some other
    architectures.

    - Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT only for 64-bit
    - Remove asm/syscalls.h UAPI header and merge to asm/unistd.h
    - Adjust kernel asm/unistd.h

    So now asm/unistd.h UAPI header should show all syscalls for riscv.

<asm/syscalls.h> may be restored by

Subject: [PATCH] riscv: restore asm/syscalls.h UAPI header
Date: Tue, 11 Dec 2018 09:09:35 +0100

UAPI header asm/syscalls.h was merged into UAPI asm/unistd.h header,
which did resolve issue with missing syscalls macros resulting in
glibc (2.28) build failure. It also broke glibc in a different way:
asm/syscalls.h is being used by glibc. I noticed this while doing
Fedora 30/Rawhide mass rebuild.

The patch returns asm/syscalls.h header and incl. it into asm/unistd.h.
I plan to send a patch to glibc to use asm/unistd.h instead of
asm/syscalls.h

In the meantime, we use __has_include__, which was added to GCC 5, to
check if <asm/syscalls.h> exists before including it.  Tested with
build-many-glibcs.py for riscv against kernel 4.19.12 and 4.20-rc7.

	[BZ #24022]
	* sysdeps/unix/sysv/linux/riscv/flush-icache.c: Check if
	<asm/syscalls.h> exists with __has_include__ before including it.
---
 sysdeps/unix/sysv/linux/riscv/flush-icache.c | 4 ++++
 1 file changed, 4 insertions(+)
Joseph Myers - Dec. 31, 2018, 5:10 p.m.
On Fri, 21 Dec 2018, H.J. Lu wrote:

> +#if __has_include__ (<asm/syscalls.h>)
>  #include <asm/syscalls.h>
> +#else
> +#include <asm/unistd.h>
> +#endif

Missing preprocessor indentation, "# include", in both halves of the #if.  
OK with that fixed, please commit, given that the asm/syscalls.h header is 
in fact not in 4.20.
Florian Weimer - Dec. 31, 2018, 5:36 p.m.
* H. J. Lu:

> +#if __has_include__ (<asm/syscalls.h>)

Isn't __has_include (no trailing __) the more canonical name?
I think that's what's in C++17.
H.J. Lu - Dec. 31, 2018, 6:17 p.m.
On Mon, Dec 31, 2018 at 9:36 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu:
>
> > +#if __has_include__ (<asm/syscalls.h>)
>
> Isn't __has_include (no trailing __) the more canonical name?
> I think that's what's in C++17.
>

I think __has_include__ is OK for glibc internal source.
Florian Weimer - Dec. 31, 2018, 6:25 p.m.
* H. J. Lu:

> On Mon, Dec 31, 2018 at 9:36 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>>
>> * H. J. Lu:
>>
>> > +#if __has_include__ (<asm/syscalls.h>)
>>
>> Isn't __has_include (no trailing __) the more canonical name?
>> I think that's what's in C++17.
>
> I think __has_include__ is OK for glibc internal source.

Both are accepted by the compiler.  But I want to reduce cognitive
load for the reader, and avoid use unportable constructs when
prefectly portable ones exist.
H.J. Lu - Dec. 31, 2018, 6:32 p.m.
On Mon, Dec 31, 2018 at 10:25 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu:
>
> > On Mon, Dec 31, 2018 at 9:36 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> > +#if __has_include__ (<asm/syscalls.h>)
> >>
> >> Isn't __has_include (no trailing __) the more canonical name?
> >> I think that's what's in C++17.
> >
> > I think __has_include__ is OK for glibc internal source.
>
> Both are accepted by the compiler.  But I want to reduce cognitive
> load for the reader, and avoid use unportable constructs when
> prefectly portable ones exist.

Fine with me.
Mark Corbin - Jan. 3, 2019, 11:59 a.m.
On 21/12/2018 17:48, H.J. Lu wrote:
> ---
>  sysdeps/unix/sysv/linux/riscv/flush-icache.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
> index d612ef4c6c..d0ecec5107 100644
> --- a/sysdeps/unix/sysv/linux/riscv/flush-icache.c
> +++ b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
> @@ -21,7 +21,11 @@
>  #include <stdlib.h>
>  #include <atomic.h>
>  #include <sys/cachectl.h>
> +#if __has_include__ (<asm/syscalls.h>)
>  #include <asm/syscalls.h>
> +#else
> +#include <asm/unistd.h>
> +#endif
>  
>  typedef int (*func_type) (void *, void *, unsigned long int);
>  

Hello

I was wondering whether it would be possible for somebody to backport
this fix (commit id 0b9c84906f653978fb8768c7ebd0ee14a47e662e) from
master to the release/2.28/master branch.

The asm/syscalls.h file has been removed for RISC-V in the 4.20 kernel
and this is currently causing glibc build failures for the RISC-V
architecture under Buildroot (https://buildroot.org). I have a patch for
glibc in Buildroot, but it would be good to use the upstream release.

Many thanks

Mark
Florian Weimer - Jan. 3, 2019, 1:51 p.m.
* Mark Corbin:

> On 21/12/2018 17:48, H.J. Lu wrote:
>> ---
>>  sysdeps/unix/sysv/linux/riscv/flush-icache.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
>> index d612ef4c6c..d0ecec5107 100644
>> --- a/sysdeps/unix/sysv/linux/riscv/flush-icache.c
>> +++ b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
>> @@ -21,7 +21,11 @@
>>  #include <stdlib.h>
>>  #include <atomic.h>
>>  #include <sys/cachectl.h>
>> +#if __has_include__ (<asm/syscalls.h>)
>>  #include <asm/syscalls.h>
>> +#else
>> +#include <asm/unistd.h>
>> +#endif
>>  
>>  typedef int (*func_type) (void *, void *, unsigned long int);
>>  
>
> Hello
>
> I was wondering whether it would be possible for somebody to backport
> this fix (commit id 0b9c84906f653978fb8768c7ebd0ee14a47e662e) from
> master to the release/2.28/master branch.
>
> The asm/syscalls.h file has been removed for RISC-V in the 4.20 kernel
> and this is currently causing glibc build failures for the RISC-V
> architecture under Buildroot (https://buildroot.org). I have a patch for
> glibc in Buildroot, but it would be good to use the upstream release.

I have now done so.

Thanks,
Florian
Mark Corbin - Jan. 3, 2019, 2:14 p.m.
On 03/01/2019 13:51, Florian Weimer wrote:
> * Mark Corbin:
>
>> On 21/12/2018 17:48, H.J. Lu wrote:
>>> ---
>>>  sysdeps/unix/sysv/linux/riscv/flush-icache.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
>>> index d612ef4c6c..d0ecec5107 100644
>>> --- a/sysdeps/unix/sysv/linux/riscv/flush-icache.c
>>> +++ b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
>>> @@ -21,7 +21,11 @@
>>>  #include <stdlib.h>
>>>  #include <atomic.h>
>>>  #include <sys/cachectl.h>
>>> +#if __has_include__ (<asm/syscalls.h>)
>>>  #include <asm/syscalls.h>
>>> +#else
>>> +#include <asm/unistd.h>
>>> +#endif
>>>  
>>>  typedef int (*func_type) (void *, void *, unsigned long int);
>>>  
>> Hello
>>
>> I was wondering whether it would be possible for somebody to backport
>> this fix (commit id 0b9c84906f653978fb8768c7ebd0ee14a47e662e) from
>> master to the release/2.28/master branch.
>>
>> The asm/syscalls.h file has been removed for RISC-V in the 4.20 kernel
>> and this is currently causing glibc build failures for the RISC-V
>> architecture under Buildroot (https://buildroot.org). I have a patch for
>> glibc in Buildroot, but it would be good to use the upstream release.
> I have now done so.
>
> Thanks,
> Florian

Thanks - appreciate it.
Palmer Dabbelt - Jan. 4, 2019, 10:56 p.m.
On Fri, 21 Dec 2018 09:48:40 PST (-0800), H.J. Lu wrote:
> <asm/syscalls.h> has been removed by
>
> commit 27f8899d6002e11a6e2d995e29b8deab5aa9cc25
> Author: David Abdurachmanov <david.abdurachmanov@gmail.com>
> Date:   Thu Nov 8 20:02:39 2018 +0100
>
>     riscv: add asm/unistd.h UAPI header
>
>     Marcin Juszkiewicz reported issues while generating syscall table for riscv
>     using 4.20-rc1. The patch refactors our unistd.h files to match some other
>     architectures.
>
>     - Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT only for 64-bit
>     - Remove asm/syscalls.h UAPI header and merge to asm/unistd.h
>     - Adjust kernel asm/unistd.h
>
>     So now asm/unistd.h UAPI header should show all syscalls for riscv.
>
> <asm/syscalls.h> may be restored by
>
> Subject: [PATCH] riscv: restore asm/syscalls.h UAPI header
> Date: Tue, 11 Dec 2018 09:09:35 +0100
>
> UAPI header asm/syscalls.h was merged into UAPI asm/unistd.h header,
> which did resolve issue with missing syscalls macros resulting in
> glibc (2.28) build failure. It also broke glibc in a different way:
> asm/syscalls.h is being used by glibc. I noticed this while doing
> Fedora 30/Rawhide mass rebuild.
>
> The patch returns asm/syscalls.h header and incl. it into asm/unistd.h.
> I plan to send a patch to glibc to use asm/unistd.h instead of
> asm/syscalls.h
>
> In the meantime, we use __has_include__, which was added to GCC 5, to
> check if <asm/syscalls.h> exists before including it.  Tested with
> build-many-glibcs.py for riscv against kernel 4.19.12 and 4.20-rc7.
>
> 	[BZ #24022]
> 	* sysdeps/unix/sysv/linux/riscv/flush-icache.c: Check if
> 	<asm/syscalls.h> exists with __has_include__ before including it.
> ---
>  sysdeps/unix/sysv/linux/riscv/flush-icache.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
> index d612ef4c6c..d0ecec5107 100644
> --- a/sysdeps/unix/sysv/linux/riscv/flush-icache.c
> +++ b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
> @@ -21,7 +21,11 @@
>  #include <stdlib.h>
>  #include <atomic.h>
>  #include <sys/cachectl.h>
> +#if __has_include__ (<asm/syscalls.h>)
>  #include <asm/syscalls.h>
> +#else
> +#include <asm/unistd.h>
> +#endif
>
>  typedef int (*func_type) (void *, void *, unsigned long int);

Thanks!

Patch

diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
index d612ef4c6c..d0ecec5107 100644
--- a/sysdeps/unix/sysv/linux/riscv/flush-icache.c
+++ b/sysdeps/unix/sysv/linux/riscv/flush-icache.c
@@ -21,7 +21,11 @@ 
 #include <stdlib.h>
 #include <atomic.h>
 #include <sys/cachectl.h>
+#if __has_include__ (<asm/syscalls.h>)
 #include <asm/syscalls.h>
+#else
+#include <asm/unistd.h>
+#endif
 
 typedef int (*func_type) (void *, void *, unsigned long int);