* semtimedop, powerpc, time64 and older kernels
@ 2020-09-30 18:01 Matheus Castanho
2020-09-30 18:29 ` Matheus Castanho
0 siblings, 1 reply; 5+ messages in thread
From: Matheus Castanho @ 2020-09-30 18:01 UTC (permalink / raw)
To: Adhemerval Zanella, GNU C Library
Hi Adhemerval,
sysvipc/test-sysvsem started failing on ppc64le on kernels older than
5.1 after:
commit aaa12e9ff02b32d5fbb2f367d7d6b6985a2176d6
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date: Fri Sep 25 15:04:34 2020 -0300
sysvipc: Fix semtimeop for !__ASSUME_DIRECT_SYSVIPC_SYSCALLS
The __NR_ipc syscall does not support 64-bit time operations. It
fixes 7c437d3778.
Checked on i686-linux-gnu on a Linux 5.4.
It fails with:
FAIL: sysvipc/test-sysvsem
original exit status 1
error: test-sysvsem.c:101: semop failed (errno=38)
error: 1 test failures
Looks like semtimedop was added on Linux 5.1, so it makes sense that
older kernels will fail with ENOSYS when calling that. So in such cases
should we also apply time convertion and fallback to semtimedop/ipc in
__semtimedop64 as done when !__ASSUME_DIRECT_SYSVIPC_SYSCALLS?
The patch below seems to solve the issue, at least on ppc64le.
Suggestions?
Thanks,
Matheus Castanho
--- 8< ---
diff --git a/sysdeps/unix/sysv/linux/semtimedop.c
b/sysdeps/unix/sysv/linux/semtimedop.c
index a9ad922ee2..510fea1852 100644
--- a/sysdeps/unix/sysv/linux/semtimedop.c
+++ b/sysdeps/unix/sysv/linux/semtimedop.c
@@ -32,7 +32,7 @@ __semtimedop64 (int semid, struct sembuf *sops, size_t
nsops,
int r = INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops,
timeout);
-#ifndef __ASSUME_TIME64_SYSCALLS
+#if !(defined __ASSUME_TIME64_SYSCALLS) || __LINUX_KERNEL_VERSION <
0x050100
if (r == 0 || errno != ENOSYS)
return r;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: semtimedop, powerpc, time64 and older kernels 2020-09-30 18:01 semtimedop, powerpc, time64 and older kernels Matheus Castanho @ 2020-09-30 18:29 ` Matheus Castanho 2020-09-30 18:45 ` Andreas Schwab 2020-09-30 19:12 ` Adhemerval Zanella 0 siblings, 2 replies; 5+ messages in thread From: Matheus Castanho @ 2020-09-30 18:29 UTC (permalink / raw) To: Adhemerval Zanella, GNU C Library [-- Attachment #1: Type: text/plain, Size: 1928 bytes --] On 9/30/20 3:01 PM, Matheus Castanho via Libc-alpha wrote: > Hi Adhemerval, > > sysvipc/test-sysvsem started failing on ppc64le on kernels older than > 5.1 after: > > commit aaa12e9ff02b32d5fbb2f367d7d6b6985a2176d6 > Author: Adhemerval Zanella <adhemerval.zanella@linaro.org> > Date: Fri Sep 25 15:04:34 2020 -0300 > > sysvipc: Fix semtimeop for !__ASSUME_DIRECT_SYSVIPC_SYSCALLS > > The __NR_ipc syscall does not support 64-bit time operations. It > fixes 7c437d3778. > > Checked on i686-linux-gnu on a Linux 5.4. > > It fails with: > FAIL: sysvipc/test-sysvsem > original exit status 1 > error: test-sysvsem.c:101: semop failed (errno=38) > error: 1 test failures > > Looks like semtimedop was added on Linux 5.1, so it makes sense that > older kernels will fail with ENOSYS when calling that. So in such cases > should we also apply time convertion and fallback to semtimedop/ipc in > __semtimedop64 as done when !__ASSUME_DIRECT_SYSVIPC_SYSCALLS? -----------------^ Sorry, I meant !_ASSUME_TIME64_SYSCALLS > > The patch below seems to solve the issue, at least on ppc64le. > > Suggestions? > > Thanks, > Matheus Castanho > > --- 8< --- > > diff --git a/sysdeps/unix/sysv/linux/semtimedop.c > b/sysdeps/unix/sysv/linux/semtimedop.c > index a9ad922ee2..510fea1852 100644 > --- a/sysdeps/unix/sysv/linux/semtimedop.c > +++ b/sysdeps/unix/sysv/linux/semtimedop.c > @@ -32,7 +32,7 @@ __semtimedop64 (int semid, struct sembuf *sops, size_t > nsops, > int r = INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops, > timeout); > > -#ifndef __ASSUME_TIME64_SYSCALLS > +#if !(defined __ASSUME_TIME64_SYSCALLS) || __LINUX_KERNEL_VERSION < > 0x050100 > if (r == 0 || errno != ENOSYS) > return r; > Also, looks like my email client messed up the diff *sigh*. I'm sending a proper patch attached this time. -- Matheus Castanho [-- Attachment #2: 0001-sysvipc-Fix-semtimedop-for-Linux-5.1.patch --] [-- Type: text/x-patch, Size: 1077 bytes --] From 1c0a497a3f986bc6980581c9eab482ccf7bb190f Mon Sep 17 00:00:00 2001 From: Matheus Castanho <msc@linux.ibm.com> Date: Wed, 30 Sep 2020 14:22:18 -0300 Subject: [PATCH] sysvipc: Fix semtimedop for Linux < 5.1 Kernels older than 5.1 will fail with ENOSYS when calling semtimedop_time64 syscall in __semtimedop_time64. Just like for !__ASSUME_TIME64_SYSCALLS, we should fallback to using the old mechanism in such cases. --- sysdeps/unix/sysv/linux/semtimedop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sysdeps/unix/sysv/linux/semtimedop.c b/sysdeps/unix/sysv/linux/semtimedop.c index a9ad922ee2..510fea1852 100644 --- a/sysdeps/unix/sysv/linux/semtimedop.c +++ b/sysdeps/unix/sysv/linux/semtimedop.c @@ -32,7 +32,7 @@ __semtimedop64 (int semid, struct sembuf *sops, size_t nsops, int r = INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops, timeout); -#ifndef __ASSUME_TIME64_SYSCALLS +#if !(defined __ASSUME_TIME64_SYSCALLS) || __LINUX_KERNEL_VERSION < 0x050100 if (r == 0 || errno != ENOSYS) return r; -- 2.26.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: semtimedop, powerpc, time64 and older kernels 2020-09-30 18:29 ` Matheus Castanho @ 2020-09-30 18:45 ` Andreas Schwab 2020-09-30 19:12 ` Adhemerval Zanella 1 sibling, 0 replies; 5+ messages in thread From: Andreas Schwab @ 2020-09-30 18:45 UTC (permalink / raw) To: Matheus Castanho via Libc-alpha On Sep 30 2020, Matheus Castanho via Libc-alpha wrote: > diff --git a/sysdeps/unix/sysv/linux/semtimedop.c b/sysdeps/unix/sysv/linux/semtimedop.c > index a9ad922ee2..510fea1852 100644 > --- a/sysdeps/unix/sysv/linux/semtimedop.c > +++ b/sysdeps/unix/sysv/linux/semtimedop.c > @@ -32,7 +32,7 @@ __semtimedop64 (int semid, struct sembuf *sops, size_t nsops, > int r = INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops, > timeout); > > -#ifndef __ASSUME_TIME64_SYSCALLS > +#if !(defined __ASSUME_TIME64_SYSCALLS) || __LINUX_KERNEL_VERSION < 0x050100 Don't use __LINUX_KERNEL_VERSION directly, instead add a new __ASSUME define in sysdeps/unix/sysv/linux/kernel-features.h and use that. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: semtimedop, powerpc, time64 and older kernels 2020-09-30 18:29 ` Matheus Castanho 2020-09-30 18:45 ` Andreas Schwab @ 2020-09-30 19:12 ` Adhemerval Zanella 2020-09-30 20:45 ` Matheus Castanho 1 sibling, 1 reply; 5+ messages in thread From: Adhemerval Zanella @ 2020-09-30 19:12 UTC (permalink / raw) To: Matheus Castanho, GNU C Library On 30/09/2020 15:29, Matheus Castanho wrote: > Also, looks like my email client messed up the diff *sigh*. I'm sending > a proper patch attached this time. > > -- > Matheus Castanho > > From 1c0a497a3f986bc6980581c9eab482ccf7bb190f Mon Sep 17 00:00:00 2001 > From: Matheus Castanho <msc@linux.ibm.com> > Date: Wed, 30 Sep 2020 14:22:18 -0300 > Subject: [PATCH] sysvipc: Fix semtimedop for Linux < 5.1 > > Kernels older than 5.1 will fail with ENOSYS when calling > semtimedop_time64 syscall in __semtimedop_time64. Just like for > !__ASSUME_TIME64_SYSCALLS, we should fallback to using the old mechanism > in such cases. > --- > sysdeps/unix/sysv/linux/semtimedop.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sysdeps/unix/sysv/linux/semtimedop.c b/sysdeps/unix/sysv/linux/semtimedop.c > index a9ad922ee2..510fea1852 100644 > --- a/sysdeps/unix/sysv/linux/semtimedop.c > +++ b/sysdeps/unix/sysv/linux/semtimedop.c > @@ -32,7 +32,7 @@ __semtimedop64 (int semid, struct sembuf *sops, size_t nsops, > int r = INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops, > timeout); > > -#ifndef __ASSUME_TIME64_SYSCALLS > +#if !(defined __ASSUME_TIME64_SYSCALLS) || __LINUX_KERNEL_VERSION < 0x050100 > if (r == 0 || errno != ENOSYS) > return r; > > -- > 2.26.2 Thanks for catching it and although it fixes the regression, we have kernel-features.h exactly to avoid using __LINUX_KERNEL_VERSION through the implementations. Also this is sub-optimal since it forces semtimeopd issues __NR_semtimeop and then __NR_ipc on powerpc64 and we have __ASSUME_DIRECT_SYSVIPC_SYSCALLS exactly to avoid this strategy of handling ENOSYS for newer syscalls and thus slowing it down the implementation on older kernels (--enable-kernel exists exactly to get rid of this older kernel support). I forgot that powerpc64 and s390x used the older multiplexed __NR_ipc and kernel v5.1 decided to add proper __NR_semtimedop (and it was in fact handled by 720e9541f5d919). I think a better fix is the one below, since it: 1. Issues __NR_semtimeop_time64 iff it is defined (32-bit architectures). 2. Issues __NR_semtimeop otherwise iff glibc is configured for a kernel that supports it (for powerpc64 it will only for --enable-kernel=5.1). Otherwise it will use only 3. 3. Issues __NR_ipc with IPCOP_semtimedop. For powerpc64 it will issue either __NR_ipc (default) or __NR_semtimeop (--enable-kernel=5.1), while for powerpc it will use either __NR_semtimeop_time64 and fallback to __NR_ipc or just issue __NR_semtimeop_time64. I am running some regressions before commit it. --- diff --git a/sysdeps/unix/sysv/linux/semtimedop.c b/sysdeps/unix/sysv/linux/semtimedop.c index a9ad922ee2..29647f8ccd 100644 --- a/sysdeps/unix/sysv/linux/semtimedop.c +++ b/sysdeps/unix/sysv/linux/semtimedop.c @@ -26,11 +26,15 @@ int __semtimedop64 (int semid, struct sembuf *sops, size_t nsops, const struct __timespec64 *timeout) { -#ifndef __NR_semtimedop_time64 -# define __NR_semtimedop_time64 __NR_semtimedop + int r; +#if defined __NR_semtimedop_time64 + r = INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops, timeout); +#elif defined __ASSUME_DIRECT_SYSVIPC_SYSCALLS && defined __NR_semtimedop + r = INLINE_SYSCALL_CALL (semtimedop, semid, sops, nsops, timeout); +#else + r = INLINE_SYSCALL_CALL (ipc, IPCOP_semtimedop, semid, + SEMTIMEDOP_IPC_ARGS (nsops, sops, timeout)); #endif - int r = INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops, - timeout); #ifndef __ASSUME_TIME64_SYSCALLS if (r == 0 || errno != ENOSYS) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: semtimedop, powerpc, time64 and older kernels 2020-09-30 19:12 ` Adhemerval Zanella @ 2020-09-30 20:45 ` Matheus Castanho 0 siblings, 0 replies; 5+ messages in thread From: Matheus Castanho @ 2020-09-30 20:45 UTC (permalink / raw) To: Adhemerval Zanella, GNU C Library On 9/30/20 4:12 PM, Adhemerval Zanella wrote: > > > On 30/09/2020 15:29, Matheus Castanho wrote: >> Also, looks like my email client messed up the diff *sigh*. I'm sending >> a proper patch attached this time. >> >> -- >> Matheus Castanho >> > >> From 1c0a497a3f986bc6980581c9eab482ccf7bb190f Mon Sep 17 00:00:00 2001 >> From: Matheus Castanho <msc@linux.ibm.com> >> Date: Wed, 30 Sep 2020 14:22:18 -0300 >> Subject: [PATCH] sysvipc: Fix semtimedop for Linux < 5.1 >> >> Kernels older than 5.1 will fail with ENOSYS when calling >> semtimedop_time64 syscall in __semtimedop_time64. Just like for >> !__ASSUME_TIME64_SYSCALLS, we should fallback to using the old mechanism >> in such cases. >> --- >> sysdeps/unix/sysv/linux/semtimedop.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/sysdeps/unix/sysv/linux/semtimedop.c b/sysdeps/unix/sysv/linux/semtimedop.c >> index a9ad922ee2..510fea1852 100644 >> --- a/sysdeps/unix/sysv/linux/semtimedop.c >> +++ b/sysdeps/unix/sysv/linux/semtimedop.c >> @@ -32,7 +32,7 @@ __semtimedop64 (int semid, struct sembuf *sops, size_t nsops, >> int r = INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops, >> timeout); >> >> -#ifndef __ASSUME_TIME64_SYSCALLS >> +#if !(defined __ASSUME_TIME64_SYSCALLS) || __LINUX_KERNEL_VERSION < 0x050100 >> if (r == 0 || errno != ENOSYS) >> return r; >> >> -- >> 2.26.2 > > Thanks for catching it and although it fixes the regression, we have > kernel-features.h exactly to avoid using __LINUX_KERNEL_VERSION through the > implementations. Also this is sub-optimal since it forces semtimeopd issues > __NR_semtimeop and then __NR_ipc on powerpc64 and we have > __ASSUME_DIRECT_SYSVIPC_SYSCALLS exactly to avoid this strategy of handling > ENOSYS for newer syscalls and thus slowing it down the implementation on > older kernels (--enable-kernel exists exactly to get rid of this older kernel > support). > Thanks for the explanation. > I forgot that powerpc64 and s390x used the older multiplexed __NR_ipc and > kernel v5.1 decided to add proper __NR_semtimedop (and it was in fact handled > by 720e9541f5d919). I think a better fix is the one below, since it: > > 1. Issues __NR_semtimeop_time64 iff it is defined (32-bit architectures). > 2. Issues __NR_semtimeop otherwise iff glibc is configured for a kernel that > supports it (for powerpc64 it will only for --enable-kernel=5.1). > Otherwise it will use only 3. > 3. Issues __NR_ipc with IPCOP_semtimedop. > > For powerpc64 it will issue either __NR_ipc (default) or __NR_semtimeop > (--enable-kernel=5.1), while for powerpc it will use either > __NR_semtimeop_time64 and fallback to __NR_ipc or just issue > __NR_semtimeop_time64. Sounds good to me. That is indeed a better solution. Thanks! > > I am running some regressions before commit it. > > --- > > diff --git a/sysdeps/unix/sysv/linux/semtimedop.c b/sysdeps/unix/sysv/linux/semtimedop.c > index a9ad922ee2..29647f8ccd 100644 > --- a/sysdeps/unix/sysv/linux/semtimedop.c > +++ b/sysdeps/unix/sysv/linux/semtimedop.c > @@ -26,11 +26,15 @@ int > __semtimedop64 (int semid, struct sembuf *sops, size_t nsops, > const struct __timespec64 *timeout) > { > -#ifndef __NR_semtimedop_time64 > -# define __NR_semtimedop_time64 __NR_semtimedop > + int r; > +#if defined __NR_semtimedop_time64 > + r = INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops, timeout); > +#elif defined __ASSUME_DIRECT_SYSVIPC_SYSCALLS && defined __NR_semtimedop > + r = INLINE_SYSCALL_CALL (semtimedop, semid, sops, nsops, timeout); > +#else > + r = INLINE_SYSCALL_CALL (ipc, IPCOP_semtimedop, semid, > + SEMTIMEDOP_IPC_ARGS (nsops, sops, timeout)); > #endif > - int r = INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops, > - timeout); > > #ifndef __ASSUME_TIME64_SYSCALLS > if (r == 0 || errno != ENOSYS) > The change looks good and fixes the issue. Tested on ppc64le. Reviewed-by: Matheus Castanho <msc@linux.ibm.com> -- Matheus Castanho ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-30 20:45 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-30 18:01 semtimedop, powerpc, time64 and older kernels Matheus Castanho 2020-09-30 18:29 ` Matheus Castanho 2020-09-30 18:45 ` Andreas Schwab 2020-09-30 19:12 ` Adhemerval Zanella 2020-09-30 20:45 ` Matheus Castanho
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).