Message ID | 20210316135612.1400708-1-stli@linux.ibm.com |
---|---|
State | Superseded |
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 5E79B3861027; Tue, 16 Mar 2021 13:56:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5E79B3861027 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1615902984; bh=tM7gItzPPiF2Lzqew8a6O+eKQMylArpusmbqv5+5VEk=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=xtaiYIfF01JFP4yAqK+858NpYgo56tzzAsXfhCjayUl/pl1oX64Dnb4ZKCKGXJ3BP AabHcHXuGwDb275bRCvx1na6C82BXKPlq68IiStQ6FqU0IeROyf7h4/vIxQtVdr2l5 xOtGMQ7xW0xbB9Ag5jnMj2NERbwTyeuiUVES5SFk= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 29C47385800F for <libc-alpha@sourceware.org>; Tue, 16 Mar 2021 13:56:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 29C47385800F Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 12GDX2WJ086666; Tue, 16 Mar 2021 09:56:19 -0400 Received: from ppma02fra.de.ibm.com (47.49.7a9f.ip4.static.sl-reverse.com [159.122.73.71]) by mx0a-001b2d01.pphosted.com with ESMTP id 37add3a412-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 16 Mar 2021 09:56:19 -0400 Received: from pps.filterd (ppma02fra.de.ibm.com [127.0.0.1]) by ppma02fra.de.ibm.com (8.16.0.43/8.16.0.43) with SMTP id 12GDqGKk009662; Tue, 16 Mar 2021 13:56:17 GMT Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by ppma02fra.de.ibm.com with ESMTP id 378n181h58-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 16 Mar 2021 13:56:17 +0000 Received: from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com [9.149.105.61]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 12GDuEx050200842 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 16 Mar 2021 13:56:14 GMT Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 2688211C052; Tue, 16 Mar 2021 13:56:14 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id F20C911C054; Tue, 16 Mar 2021 13:56:13 +0000 (GMT) Received: from t3560005.lnxne.boe (unknown [9.152.108.100]) by d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP; Tue, 16 Mar 2021 13:56:13 +0000 (GMT) To: libc-alpha@sourceware.org Subject: [PATCH] Don't test nanoseconds for non-LFS interface in io/tst-stat.c Date: Tue, 16 Mar 2021 14:56:12 +0100 Message-Id: <20210316135612.1400708-1-stli@linux.ibm.com> X-Mailer: git-send-email 2.28.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.369, 18.0.761 definitions=2021-03-16_04:2021-03-16, 2021-03-16 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 phishscore=0 lowpriorityscore=0 spamscore=0 malwarescore=0 suspectscore=0 adultscore=0 priorityscore=1501 mlxscore=0 mlxlogscore=861 impostorscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2103160093 X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, 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: Stefan Liebler via Libc-alpha <libc-alpha@sourceware.org> Reply-To: Stefan Liebler <stli@linux.ibm.com> Cc: Stefan Liebler <stli@linux.ibm.com> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
Don't test nanoseconds for non-LFS interface in io/tst-stat.c
|
|
Commit Message
Stefan Liebler
March 16, 2021, 1:56 p.m. UTC
Both new tests io/tst-stat and io/tst-stat-lfs (_FILE_OFFSET_BITS=64) are comparing the nanosecond fields with the statx result. Unfortunately e.g. on s390(31bit) those fields are always zero if old KABI with non-LFS support is used. With _FILE_OFFSET_BITS=64 stat is using statx internally. As suggested by Adhemerval this patch disables the nanosecond check for non-LFS interface. --- io/tst-stat.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
Comments
On 16/03/2021 10:56, Stefan Liebler wrote: > Both new tests io/tst-stat and io/tst-stat-lfs (_FILE_OFFSET_BITS=64) > are comparing the nanosecond fields with the statx result. Unfortunately > e.g. on s390(31bit) those fields are always zero if old KABI with non-LFS > support is used. With _FILE_OFFSET_BITS=64 stat is using statx internally. This might also happens for LFS interface if statx is not supported by the kernel, since the LFS call will fall back to the use the stat syscall that has this issue. Maybe it would be better to make it an internal tests and add a flag somewhere to just disable it for s390-32. > > As suggested by Adhemerval this patch disables the nanosecond check for > non-LFS interface. > --- > io/tst-stat.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/io/tst-stat.c b/io/tst-stat.c > index 445ac4176c..550128a2a5 100644 > --- a/io/tst-stat.c > +++ b/io/tst-stat.c > @@ -26,6 +26,20 @@ > #include <sys/stat.h> > #include <sys/sysmacros.h> > #include <unistd.h> > +#include <kernel_stat.h> > + > +#if defined _FILE_OFFSET_BITS && _FILE_OFFSET_BITS == 64 > +/* Test nanoseconds if LFS interface is requested. */ > +# define TEST_NANOSECONDS 1 > +#elif !XSTAT_IS_XSTAT64 > +/* LFS interface is not requested and we have no LFS support. E.g. s390(31bit) > + is using old KABI with old non-LFS support and the nanoseconds fields are > + always zero. */ > +# define TEST_NANOSECONDS 0 > +#else > +/* LFS interface is not requested, but we have LFS support. */ > +# define TEST_NANOSECONDS 1 > +#endif > > static void > stat_check (int fd, const char *path, struct stat *st) > @@ -91,9 +105,11 @@ do_test (void) > TEST_COMPARE (stx.stx_blocks, st.st_blocks); > > TEST_COMPARE (stx.stx_ctime.tv_sec, st.st_ctim.tv_sec); > - TEST_COMPARE (stx.stx_ctime.tv_nsec, st.st_ctim.tv_nsec); > TEST_COMPARE (stx.stx_mtime.tv_sec, st.st_mtim.tv_sec); > +#if TEST_NANOSECONDS > + TEST_COMPARE (stx.stx_ctime.tv_nsec, st.st_ctim.tv_nsec); > TEST_COMPARE (stx.stx_mtime.tv_nsec, st.st_mtim.tv_nsec); > +#endif > } > > return 0; >
On 16/03/2021 15:24, Adhemerval Zanella wrote: > > > On 16/03/2021 10:56, Stefan Liebler wrote: >> Both new tests io/tst-stat and io/tst-stat-lfs (_FILE_OFFSET_BITS=64) >> are comparing the nanosecond fields with the statx result. Unfortunately >> e.g. on s390(31bit) those fields are always zero if old KABI with non-LFS >> support is used. With _FILE_OFFSET_BITS=64 stat is using statx internally. > > This might also happens for LFS interface if statx is not supported by the > kernel, since the LFS call will fall back to the use the stat syscall that > has this issue. > > Maybe it would be better to make it an internal tests and add a flag > somewhere to just disable it for s390-32. > gcc is setting the macros __s390x__ and __s390__ on s390x(64bit), but only __s390__ on s390(31bit). Thus I can detect s390(31bit) at compile-time via "#if" and disable the nanoseconds checks on s390(31bit) at all and run it on all other cases. Then I don't need to make the test an internal test and don't need special flags. If this is okay for you, I will prepare a further patch (also with a different subject). Thanks, Stefan
On 16/03/2021 13:30, Stefan Liebler wrote: > On 16/03/2021 15:24, Adhemerval Zanella wrote: >> >> >> On 16/03/2021 10:56, Stefan Liebler wrote: >>> Both new tests io/tst-stat and io/tst-stat-lfs (_FILE_OFFSET_BITS=64) >>> are comparing the nanosecond fields with the statx result. Unfortunately >>> e.g. on s390(31bit) those fields are always zero if old KABI with non-LFS >>> support is used. With _FILE_OFFSET_BITS=64 stat is using statx internally. >> >> This might also happens for LFS interface if statx is not supported by the >> kernel, since the LFS call will fall back to the use the stat syscall that >> has this issue. >> >> Maybe it would be better to make it an internal tests and add a flag >> somewhere to just disable it for s390-32. >> > gcc is setting the macros __s390x__ and __s390__ on s390x(64bit), but > only __s390__ on s390(31bit). Thus I can detect s390(31bit) at > compile-time via "#if" and disable the nanoseconds checks on s390(31bit) > at all and run it on all other cases. Then I don't need to make the test > an internal test and don't need special flags. If this is okay for you, > I will prepare a further patch (also with a different subject). I think it would better to add such logic on libsupport instead directly on the test itself, similar to what support_path_support_time64 does. Maybe something like: bool support_stat_nanoseconds (void) { /* s390 stat64 compat symbol does not support nanoseconds resolution and it used on non-LFS [f,l]stat[at] implementations. */ #if defined __linux__ && !defined __s390x__ && defined __s390__ return false; #else return true; } Another possibility is if you want to fix it for s390 is to call statx on Linux non-LFS fstatat call (sysdeps/unix/sysv/linux/fstatat.c) which should fix stat, lstat, and fstat as well. You will need to add a stat to statx conversion at sysdeps/unix/sysv/linux/statx_cp.c which would need handle EOVERFLOW on st_ino, st_size, and st_blocks.
On 16/03/2021 20:59, Adhemerval Zanella wrote: > > > On 16/03/2021 13:30, Stefan Liebler wrote: >> On 16/03/2021 15:24, Adhemerval Zanella wrote: >>> >>> >>> On 16/03/2021 10:56, Stefan Liebler wrote: >>>> Both new tests io/tst-stat and io/tst-stat-lfs (_FILE_OFFSET_BITS=64) >>>> are comparing the nanosecond fields with the statx result. Unfortunately >>>> e.g. on s390(31bit) those fields are always zero if old KABI with non-LFS >>>> support is used. With _FILE_OFFSET_BITS=64 stat is using statx internally. >>> >>> This might also happens for LFS interface if statx is not supported by the >>> kernel, since the LFS call will fall back to the use the stat syscall that >>> has this issue. >>> >>> Maybe it would be better to make it an internal tests and add a flag >>> somewhere to just disable it for s390-32. >>> >> gcc is setting the macros __s390x__ and __s390__ on s390x(64bit), but >> only __s390__ on s390(31bit). Thus I can detect s390(31bit) at >> compile-time via "#if" and disable the nanoseconds checks on s390(31bit) >> at all and run it on all other cases. Then I don't need to make the test >> an internal test and don't need special flags. If this is okay for you, >> I will prepare a further patch (also with a different subject). > > I think it would better to add such logic on libsupport instead directly > on the test itself, similar to what support_path_support_time64 does. > Maybe something like: > > bool > support_stat_nanoseconds (void) > { > /* s390 stat64 compat symbol does not support nanoseconds resolution > and it used on non-LFS [f,l]stat[at] implementations. */ > #if defined __linux__ && !defined __s390x__ && defined __s390__ > return false; > #else > return true; > } > > Another possibility is if you want to fix it for s390 is to call > statx on Linux non-LFS fstatat call (sysdeps/unix/sysv/linux/fstatat.c) > which should fix stat, lstat, and fstat as well. You will need to > add a stat to statx conversion at sysdeps/unix/sysv/linux/statx_cp.c > which would need handle EOVERFLOW on st_ino, st_size, and st_blocks. > I'm fine with just disabling the nanoseconds test on s390(31bit). I've also talked to the kernel guys. They won't fix the compat layer in this case. Please have a look at the new patch "[PATCH] S390: Don't test nanoseconds in io/tst-stat.c" https://sourceware.org/pipermail/libc-alpha/2021-March/124063.html Thanks, Stefan
diff --git a/io/tst-stat.c b/io/tst-stat.c index 445ac4176c..550128a2a5 100644 --- a/io/tst-stat.c +++ b/io/tst-stat.c @@ -26,6 +26,20 @@ #include <sys/stat.h> #include <sys/sysmacros.h> #include <unistd.h> +#include <kernel_stat.h> + +#if defined _FILE_OFFSET_BITS && _FILE_OFFSET_BITS == 64 +/* Test nanoseconds if LFS interface is requested. */ +# define TEST_NANOSECONDS 1 +#elif !XSTAT_IS_XSTAT64 +/* LFS interface is not requested and we have no LFS support. E.g. s390(31bit) + is using old KABI with old non-LFS support and the nanoseconds fields are + always zero. */ +# define TEST_NANOSECONDS 0 +#else +/* LFS interface is not requested, but we have LFS support. */ +# define TEST_NANOSECONDS 1 +#endif static void stat_check (int fd, const char *path, struct stat *st) @@ -91,9 +105,11 @@ do_test (void) TEST_COMPARE (stx.stx_blocks, st.st_blocks); TEST_COMPARE (stx.stx_ctime.tv_sec, st.st_ctim.tv_sec); - TEST_COMPARE (stx.stx_ctime.tv_nsec, st.st_ctim.tv_nsec); TEST_COMPARE (stx.stx_mtime.tv_sec, st.st_mtim.tv_sec); +#if TEST_NANOSECONDS + TEST_COMPARE (stx.stx_ctime.tv_nsec, st.st_ctim.tv_nsec); TEST_COMPARE (stx.stx_mtime.tv_nsec, st.st_mtim.tv_nsec); +#endif } return 0;