| Message ID | 20181221174840.10933-1-hjl.tools@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: Use __has_include__ to include <asm/syscalls.h> [BZ #24022] | expand |
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.
* 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.
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.
* 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.
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.
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
* 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
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.
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!
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);
<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(+)