Message ID | 6f370467-8d0a-bd3d-29b2-2fefef4e475f@linux.ibm.com |
---|---|
State | Dropped |
Headers |
Return-Path: <libc-alpha-bounces@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 264033870878; Wed, 30 Sep 2020 18:01:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 264033870878 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1601488896; bh=+sJEC+54/HZmYle2WveHNB4UiHrXO1OcZSLmmEGHYZ0=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=YotFADAs2FfXqOe7dM+fUsF9XE69OPHtmD6ZnXE2Gco/o61PKpdGAp/R818Gam/E2 RUkmL+pV+0VRSCa2MBeDdKaR/Y50qYWEvYRaQ5Hk8R8zJnFNyY6608/zH1NBN5RION M3r2OcQ082P+dIEPBMn7jpzNVtpfEEw2vjLZGA6U= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 16C3B3861823 for <libc-alpha@sourceware.org>; Wed, 30 Sep 2020 18:01:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 16C3B3861823 Received: from pps.filterd (m0098417.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 08UHXQlu155238; Wed, 30 Sep 2020 14:01:32 -0400 Received: from ppma04dal.us.ibm.com (7a.29.35a9.ip4.static.sl-reverse.com [169.53.41.122]) by mx0a-001b2d01.pphosted.com with ESMTP id 33vw7vkc8p-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 30 Sep 2020 14:01:32 -0400 Received: from pps.filterd (ppma04dal.us.ibm.com [127.0.0.1]) by ppma04dal.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 08UHw4ob017037; Wed, 30 Sep 2020 18:01:31 GMT Received: from b01cxnp23032.gho.pok.ibm.com (b01cxnp23032.gho.pok.ibm.com [9.57.198.27]) by ppma04dal.us.ibm.com with ESMTP id 33sw99hbq1-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 30 Sep 2020 18:01:31 +0000 Received: from b01ledav005.gho.pok.ibm.com (b01ledav005.gho.pok.ibm.com [9.57.199.110]) by b01cxnp23032.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 08UI1Uwq53805418 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 30 Sep 2020 18:01:30 GMT Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id B7AC2AE064; Wed, 30 Sep 2020 18:01:30 +0000 (GMT) Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 13184AE05C; Wed, 30 Sep 2020 18:01:30 +0000 (GMT) Received: from [9.65.230.4] (unknown [9.65.230.4]) by b01ledav005.gho.pok.ibm.com (Postfix) with ESMTP; Wed, 30 Sep 2020 18:01:29 +0000 (GMT) To: Adhemerval Zanella <adhemerval.zanella@linaro.org>, GNU C Library <libc-alpha@sourceware.org> Subject: semtimedop, powerpc, time64 and older kernels Message-ID: <6f370467-8d0a-bd3d-29b2-2fefef4e475f@linux.ibm.com> Date: Wed, 30 Sep 2020 15:01:28 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235, 18.0.687 definitions=2020-09-30_09:2020-09-30, 2020-09-30 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 lowpriorityscore=0 bulkscore=0 impostorscore=0 spamscore=0 priorityscore=1501 suspectscore=0 phishscore=0 mlxscore=0 clxscore=1015 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2006250000 definitions=main-2009300138 X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> From: Matheus Castanho via Libc-alpha <libc-alpha@sourceware.org> Reply-To: Matheus Castanho <msc@linux.ibm.com> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
semtimedop, powerpc, time64 and older kernels
|
|
Commit Message
Matheus Castanho
Sept. 30, 2020, 6:01 p.m. UTC
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< --- return r;
Comments
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
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.
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)
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
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)