diff mbox series

Define __NR_futex to be __NR_futex_time64 on riscv32

Message ID 20200429033511.1848449-1-raj.khem@gmail.com
State Changes Requested
Headers show
Series Define __NR_futex to be __NR_futex_time64 on riscv32 | expand

Commit Message

Khem Raj April 29, 2020, 3:35 a.m. UTC
RISCV glibc has decided to use 64bit time_t from get go unlike
other 32bit architecture therefore aliasing __NR_futex to
__NR_futex_time64 helps avoid the below errors on rv32

tst_checkpoint.c:99:17: error: use of undeclared identifier 'SYS_futex'

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 lib/tst_checkpoint.c                        | 4 ++++
 testcases/kernel/syscalls/clone/clone08.c   | 4 ++++
 testcases/kernel/syscalls/futex/futextest.h | 4 ++++
 3 files changed, 12 insertions(+)

Comments

Petr Vorel April 29, 2020, 1:16 p.m. UTC | #1
Hi Khem,

> RISCV glibc has decided to use 64bit time_t from get go unlike
> other 32bit architecture therefore aliasing __NR_futex to
> __NR_futex_time64 helps avoid the below errors on rv32

> tst_checkpoint.c:99:17: error: use of undeclared identifier 'SYS_futex'

Thanks for your fix.

BTW, out of curiosity, is Risc-v 32 bit merged into glibc master?

I found a patch from Alistair Francis from January, which implements what you claim:
https://sourceware.org/legacy-ml/libc-alpha/2020-01/msg00205.html
...
+/* RV32 and RV64 both use 64-bit time_t */
+#define __TIMESIZE	64
diff --git a/sysdeps/unix/sysv/linux/riscv/bits/typesizes.h b/sysdeps/unix/sysv/linux/riscv/bits/typesizes.h
...

(part of https://patches-gcc.linaro.org/project/glibc/list/?series=21554
patchset) but it hasn't been merged yet to master.

+ There is older patchset from Zong Li, not yet merged.
https://patches-gcc.linaro.org/cover/12952/

> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>  lib/tst_checkpoint.c                        | 4 ++++
>  testcases/kernel/syscalls/clone/clone08.c   | 4 ++++
>  testcases/kernel/syscalls/futex/futextest.h | 4 ++++
>  3 files changed, 12 insertions(+)

> diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
> index 5e5b11496c..0388e9db2f 100644
> --- a/lib/tst_checkpoint.c
> +++ b/lib/tst_checkpoint.c
> @@ -21,6 +21,10 @@
>   * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>   */

> +#if !defined(__NR_futex) && defined(__riscv) && __riscv_xlen == 32
> +# define __NR_futex __NR_futex_time64
> +#endif

I guess this should go to include/lapi/futex.h, so we don't repeat ourselves.
(and clone08.c needs to include it, others already do).

...

Kind regards,
Petr
Petr Vorel April 29, 2020, 1:34 p.m. UTC | #2
Hi Khem,

> > +#if !defined(__NR_futex) && defined(__riscv) && __riscv_xlen == 32
> > +# define __NR_futex __NR_futex_time64
> > +#endif

> I guess this should go to include/lapi/futex.h, so we don't repeat ourselves.
> (and clone08.c needs to include it, others already do).

I've noticed that we still don't have risc-v support in include/lapi/syscalls/.
IMHO adding it would be better, than fixing just one missing __NR numbers.

Kind regards,
Petr
Khem Raj April 29, 2020, 1:42 p.m. UTC | #3
On Wed, Apr 29, 2020 at 6:16 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Khem,
>
> > RISCV glibc has decided to use 64bit time_t from get go unlike
> > other 32bit architecture therefore aliasing __NR_futex to
> > __NR_futex_time64 helps avoid the below errors on rv32
>
> > tst_checkpoint.c:99:17: error: use of undeclared identifier 'SYS_futex'
>
> Thanks for your fix.
>
> BTW, out of curiosity, is Risc-v 32 bit merged into glibc master?

Not yet but its almost finalised and should be merged before 2.32 hopefully

>
> I found a patch from Alistair Francis from January, which implements what you claim:
> https://sourceware.org/legacy-ml/libc-alpha/2020-01/msg00205.html
> ...
> +/* RV32 and RV64 both use 64-bit time_t */
> +#define __TIMESIZE     64
> diff --git a/sysdeps/unix/sysv/linux/riscv/bits/typesizes.h b/sysdeps/unix/sysv/linux/riscv/bits/typesizes.h
> ...
>

right

> (part of https://patches-gcc.linaro.org/project/glibc/list/?series=21554
> patchset) but it hasn't been merged yet to master.

Its being reviewed

>
> + There is older patchset from Zong Li, not yet merged.
> https://patches-gcc.linaro.org/cover/12952/
>

above patches are newer versions of this one.

> > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > ---
> >  lib/tst_checkpoint.c                        | 4 ++++
> >  testcases/kernel/syscalls/clone/clone08.c   | 4 ++++
> >  testcases/kernel/syscalls/futex/futextest.h | 4 ++++
> >  3 files changed, 12 insertions(+)
>
> > diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
> > index 5e5b11496c..0388e9db2f 100644
> > --- a/lib/tst_checkpoint.c
> > +++ b/lib/tst_checkpoint.c
> > @@ -21,6 +21,10 @@
> >   * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> >   */
>
> > +#if !defined(__NR_futex) && defined(__riscv) && __riscv_xlen == 32
> > +# define __NR_futex __NR_futex_time64
> > +#endif
>
> I guess this should go to include/lapi/futex.h, so we don't repeat ourselves.
> (and clone08.c needs to include it, others already do).
>

OK makes sense, will send v2 ?

> ...
>
> Kind regards,
> Petr
Petr Vorel May 5, 2020, 12:15 p.m. UTC | #4
Hi Khem,

sorry for a delay.
> > > RISCV glibc has decided to use 64bit time_t from get go unlike
> > > other 32bit architecture therefore aliasing __NR_futex to
> > > __NR_futex_time64 helps avoid the below errors on rv32

> > > tst_checkpoint.c:99:17: error: use of undeclared identifier 'SYS_futex'

> > Thanks for your fix.

> > BTW, out of curiosity, is Risc-v 32 bit merged into glibc master?

> Not yet but its almost finalised and should be merged before 2.32 hopefully
Nice!

> > I guess this should go to include/lapi/futex.h, so we don't repeat ourselves.
> > (and clone08.c needs to include it, others already do).


> OK makes sense, will send v2 ?
Yes please.

Kind regards,
Petr
diff mbox series

Patch

diff --git a/lib/tst_checkpoint.c b/lib/tst_checkpoint.c
index 5e5b11496c..0388e9db2f 100644
--- a/lib/tst_checkpoint.c
+++ b/lib/tst_checkpoint.c
@@ -21,6 +21,10 @@ 
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
+#if !defined(__NR_futex) && defined(__riscv) && __riscv_xlen == 32
+# define __NR_futex __NR_futex_time64
+#endif
+
 #include <stdint.h>
 #include <limits.h>
 #include <errno.h>
diff --git a/testcases/kernel/syscalls/clone/clone08.c b/testcases/kernel/syscalls/clone/clone08.c
index aace308068..85a2bd9246 100644
--- a/testcases/kernel/syscalls/clone/clone08.c
+++ b/testcases/kernel/syscalls/clone/clone08.c
@@ -5,6 +5,10 @@ 
  * Author: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
  */
 
+#if !defined(__NR_futex) && defined(__riscv) && __riscv_xlen == 32
+# define __NR_futex __NR_futex_time64
+#endif
+
 #define _GNU_SOURCE
 #include <stdlib.h>
 #include <stdio.h>
diff --git a/testcases/kernel/syscalls/futex/futextest.h b/testcases/kernel/syscalls/futex/futextest.h
index 5754d36dae..59d877e30f 100644
--- a/testcases/kernel/syscalls/futex/futextest.h
+++ b/testcases/kernel/syscalls/futex/futextest.h
@@ -34,6 +34,10 @@ 
 #ifndef _FUTEXTEST_H
 #define _FUTEXTEST_H
 
+#if !defined(__NR_futex) && defined(__riscv) && __riscv_xlen == 32
+# define __NR_futex __NR_futex_time64
+#endif
+
 #include <unistd.h>
 #include <sys/syscall.h>
 #include <sys/types.h>