Message ID | 20210314164625.9312-1-lukma@denx.de |
---|---|
State | Committed |
Delegated to: | Adhemerval Zanella Netto |
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 B19E63836C17; Sun, 14 Mar 2021 16:46:48 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-out.m-online.net (mail-out.m-online.net [212.18.0.9]) by sourceware.org (Postfix) with ESMTPS id 285AE386103B; Sun, 14 Mar 2021 16:46:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 285AE386103B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: sourceware.org; spf=none smtp.mailfrom=lukma@denx.de Received: from frontend01.mail.m-online.net (unknown [192.168.8.182]) by mail-out.m-online.net (Postfix) with ESMTP id 4Dz56m51wrz1qs38; Sun, 14 Mar 2021 17:46:40 +0100 (CET) Received: from localhost (dynscan1.mnet-online.de [192.168.6.70]) by mail.m-online.net (Postfix) with ESMTP id 4Dz56m3ZBCz1qqkj; Sun, 14 Mar 2021 17:46:40 +0100 (CET) X-Virus-Scanned: amavisd-new at mnet-online.de Received: from mail.mnet-online.de ([192.168.8.182]) by localhost (dynscan1.mail.m-online.net [192.168.6.70]) (amavisd-new, port 10024) with ESMTP id 2-NJfcsCm7v1; Sun, 14 Mar 2021 17:46:38 +0100 (CET) X-Auth-Info: dYXqvCYKba5v5tCmZTJqK3PFO9Bh9lToO6lFDOENIww= Received: from localhost.localdomain (85-222-111-42.dynamic.chello.pl [85.222.111.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.mnet-online.de (Postfix) with ESMTPSA; Sun, 14 Mar 2021 17:46:38 +0100 (CET) From: Lukasz Majewski <lukma@denx.de> To: Joseph Myers <joseph@codesourcery.com>, Adhemerval Zanella <adhemerval.zanella@linaro.org>, Florian Weimer <fweimer@redhat.com>, DJ Delorie <dj@redhat.com> Subject: [PATCH] tst: Provide test for select Date: Sun, 14 Mar 2021 17:46:25 +0100 Message-Id: <20210314164625.9312-1-lukma@denx.de> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-14.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, 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> Cc: GNU C Library <libc-alpha@sourceware.org>, libc-help@sourceware.org, Florian Weimer <fw@deneb.enyo.de>, Alistair Francis <alistair.francis@wdc.com> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
tst: Provide test for select
|
|
Commit Message
Lukasz Majewski
March 14, 2021, 4:46 p.m. UTC
This change adds new test to assess select()'s timeout related functionality (the rdfs set provides valid fd - stderr - but during normal program operation there is no data to be read, so one just waits for timeout). To be more specific - two use cases are checked: - if select() times out immediately when passed struct timeval has zero values of tv_usec and tv_sec. - if select() times out after timeout specified in passed argument --- misc/Makefile | 2 +- misc/tst-select.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 misc/tst-select.c
Comments
Hi DJ, > By the way, the proper address to send patches is *just* libc-alpha. > Please do not send patches to the entire project plus multple mailing > lists. > > https://sourceware.org/glibc/wiki/Contribution%20checklist#Emailing_your_patch > I've made a mistake - the libc-help was from my last sent patch with git-send. And I do prefer to CC directly people who are involved - so they are aware of the work status. > > To: Joseph Myers <joseph@codesourcery.com>, > > Adhemerval Zanella <adhemerval.zanella@linaro.org>, > > Florian Weimer <fweimer@redhat.com>, DJ Delorie > > <dj@redhat.com> Cc: Paul Eggert <eggert@cs.ucla.edu>, Alistair > > Francis <alistair23@gmail.com>, Arnd Bergmann <arnd@arndb.de>, > > Alistair Francis <alistair.francis@wdc.com>, > > GNU C Library <libc-alpha@sourceware.org>, > > "Carlos O'Donell" <carlos@redhat.com>, > > Florian Weimer <fw@deneb.enyo.de>, Zack Weinberg > > <zackw@panix.com>, libc-help@sourceware.org, Lukasz Majewski > > <lukma@denx.de> > Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Dear Community, > This change adds new test to assess select()'s timeout related > functionality (the rdfs set provides valid fd - stderr - but during > normal program operation there is no data to be read, so one just > waits for timeout). > > To be more specific - two use cases are checked: > - if select() times out immediately when passed struct timeval has > zero values of tv_usec and tv_sec. > - if select() times out after timeout specified in passed argument Do you have any comments regarding this patch? > --- > misc/Makefile | 2 +- > misc/tst-select.c | 70 > +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 > insertions(+), 1 deletion(-) create mode 100644 misc/tst-select.c > > diff --git a/misc/Makefile b/misc/Makefile > index a8363d4b76..a3545f047e 100644 > --- a/misc/Makefile > +++ b/misc/Makefile > @@ -88,7 +88,7 @@ tests := tst-dirname tst-tsearch tst-fdset > tst-mntent tst-hsearch \ tst-preadvwritev tst-preadvwritev64 > tst-makedev tst-empty \ tst-preadvwritev2 tst-preadvwritev64v2 > tst-warn-wide \ tst-ldbl-warn tst-ldbl-error tst-dbl-efgcvt > tst-ldbl-efgcvt \ > - tst-mntent-autofs tst-syscalls tst-mntent-escape > + tst-mntent-autofs tst-syscalls tst-mntent-escape tst-select > > tests-time64 := tst-pselect-time64 > > diff --git a/misc/tst-select.c b/misc/tst-select.c > new file mode 100644 > index 0000000000..ea776c8880 > --- /dev/null > +++ b/misc/tst-select.c > @@ -0,0 +1,70 @@ > +/* Test for select timeout > + Copyright (C) 2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be > useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <time.h> > +#include <errno.h> > +#include <stdbool.h> > +#include <sys/select.h> > +#include <support/check.h> > +#include <support/xtime.h> > +#include <support/timespec.h> > + > +#define TST_SELECT_TIMEOUT 1 > +#define TST_SELECT_FD_ERR 2 > + > +static int test_select_timeout (bool zero_tmo) > +{ > + const int fds = TST_SELECT_FD_ERR; > + int timeout = TST_SELECT_TIMEOUT; > + struct timeval to = { 0, 0 }; > + struct timespec ts; > + fd_set rfds; > + > + FD_ZERO (&rfds); > + FD_SET (fds, &rfds); > + > + if (zero_tmo) > + timeout = 0; > + > + to.tv_sec = timeout; > + ts = xclock_now (CLOCK_REALTIME); > + ts = timespec_add (ts, (struct timespec) { timeout, 0 }); > + > + /* Wait for timeout. */ > + int ret = select (fds + 1, &rfds, NULL, NULL, &to); > + if (ret == -1) > + FAIL_EXIT1 ("select failed: %m\n"); > + > + TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts); > + > + return 0; > +} > + > +static int > +do_test (void) > +{ > + /* Check if select exits immediately. */ > + test_select_timeout (true); > + > + /* Check if select exits after specified timeout. */ > + test_select_timeout (false); > + > + return 0; > +} > + > +#include <support/test-driver.c> Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On 14/03/2021 13:46, Lukasz Majewski wrote: > This change adds new test to assess select()'s timeout related > functionality (the rdfs set provides valid fd - stderr - but during > normal program operation there is no data to be read, so one just > waits for timeout). > > To be more specific - two use cases are checked: > - if select() times out immediately when passed struct timeval has > zero values of tv_usec and tv_sec. > - if select() times out after timeout specified in passed argument LGTM with the nits below. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > misc/Makefile | 2 +- > misc/tst-select.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 71 insertions(+), 1 deletion(-) > create mode 100644 misc/tst-select.c > > diff --git a/misc/Makefile b/misc/Makefile > index a8363d4b76..a3545f047e 100644 > --- a/misc/Makefile > +++ b/misc/Makefile > @@ -88,7 +88,7 @@ tests := tst-dirname tst-tsearch tst-fdset tst-mntent tst-hsearch \ > tst-preadvwritev tst-preadvwritev64 tst-makedev tst-empty \ > tst-preadvwritev2 tst-preadvwritev64v2 tst-warn-wide \ > tst-ldbl-warn tst-ldbl-error tst-dbl-efgcvt tst-ldbl-efgcvt \ > - tst-mntent-autofs tst-syscalls tst-mntent-escape > + tst-mntent-autofs tst-syscalls tst-mntent-escape tst-select > > tests-time64 := tst-pselect-time64 > Ok. > diff --git a/misc/tst-select.c b/misc/tst-select.c > new file mode 100644 > index 0000000000..ea776c8880 > --- /dev/null > +++ b/misc/tst-select.c > @@ -0,0 +1,70 @@ > +/* Test for select timeout Missing period. > + Copyright (C) 2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <time.h> > +#include <errno.h> > +#include <stdbool.h> > +#include <sys/select.h> > +#include <support/check.h> > +#include <support/xtime.h> > +#include <support/timespec.h> > + > +#define TST_SELECT_TIMEOUT 1 > +#define TST_SELECT_FD_ERR 2 > + > +static int test_select_timeout (bool zero_tmo) Move the function to next line: | static int | test_select_timeout (bool zero_tmo) > +{ > + const int fds = TST_SELECT_FD_ERR; > + int timeout = TST_SELECT_TIMEOUT; > + struct timeval to = { 0, 0 }; > + struct timespec ts; > + fd_set rfds; > + > + FD_ZERO (&rfds); > + FD_SET (fds, &rfds); > + > + if (zero_tmo) > + timeout = 0; > + > + to.tv_sec = timeout; > + ts = xclock_now (CLOCK_REALTIME); > + ts = timespec_add (ts, (struct timespec) { timeout, 0 }); > + > + /* Wait for timeout. */ > + int ret = select (fds + 1, &rfds, NULL, NULL, &to); > + if (ret == -1) > + FAIL_EXIT1 ("select failed: %m\n"); > + > + TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts); > + > + return 0; > +} > + Ok. > +static int > +do_test (void) > +{ > + /* Check if select exits immediately. */ > + test_select_timeout (true); > + > + /* Check if select exits after specified timeout. */ > + test_select_timeout (false); > + > + return 0; > +} > + > +#include <support/test-driver.c> > Ok.
On Mon, Mar 22, 2021 at 4:33 AM Lukasz Majewski <lukma@denx.de> wrote: > > Dear Community, > > > This change adds new test to assess select()'s timeout related > > functionality (the rdfs set provides valid fd - stderr - but during > > normal program operation there is no data to be read, so one just > > waits for timeout). > > > > To be more specific - two use cases are checked: > > - if select() times out immediately when passed struct timeval has > > zero values of tv_usec and tv_sec. > > - if select() times out after timeout specified in passed argument > > Do you have any comments regarding this patch? > This test failed on machines with more than 40 cores: tst-select.c:54: 1616610088.851713938s not after 1616610089.851712804s (difference 0.999998866s) error: 1 test failures I was doing 3 "makec -j28 check" in parallel. H.J.
On 24/03/2021 16:17, H.J. Lu wrote: > On Mon, Mar 22, 2021 at 4:33 AM Lukasz Majewski <lukma@denx.de> wrote: >> >> Dear Community, >> >>> This change adds new test to assess select()'s timeout related >>> functionality (the rdfs set provides valid fd - stderr - but during >>> normal program operation there is no data to be read, so one just >>> waits for timeout). >>> >>> To be more specific - two use cases are checked: >>> - if select() times out immediately when passed struct timeval has >>> zero values of tv_usec and tv_sec. >>> - if select() times out after timeout specified in passed argument >> >> Do you have any comments regarding this patch? >> > > This test failed on machines with more than 40 cores: > > tst-select.c:54: 1616610088.851713938s not after 1616610089.851712804s > (difference 0.999998866s) > error: 1 test failures > > I was doing 3 "makec -j28 check" in parallel. I think the nanosecond precision of time accounting is triggering the failure, since select only support timeval (the error indicates that the nanosecond precision is what is triggering it). Maybe if we ignore the nanosecond precision: diff --git a/misc/tst-select.c b/misc/tst-select.c index 7c310256c5..4b1791ac8a 100644 --- a/misc/tst-select.c +++ b/misc/tst-select.c @@ -45,6 +45,8 @@ test_select_timeout (bool zero_tmo) to.tv_sec = timeout; ts = xclock_now (CLOCK_REALTIME); ts = timespec_add (ts, (struct timespec) { timeout, 0 }); + /* Ignore nanosecond precision since select only support microsecond. */ + ts.tv_nsec = (ts.tv_nsec * 1000) / 1000; /* Wait for timeout. */ int ret = select (fds + 1, &rfds, NULL, NULL, &to);
On Wed, Mar 24, 2021 at 12:47 PM Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > > > On 24/03/2021 16:17, H.J. Lu wrote: > > On Mon, Mar 22, 2021 at 4:33 AM Lukasz Majewski <lukma@denx.de> wrote: > >> > >> Dear Community, > >> > >>> This change adds new test to assess select()'s timeout related > >>> functionality (the rdfs set provides valid fd - stderr - but during > >>> normal program operation there is no data to be read, so one just > >>> waits for timeout). > >>> > >>> To be more specific - two use cases are checked: > >>> - if select() times out immediately when passed struct timeval has > >>> zero values of tv_usec and tv_sec. > >>> - if select() times out after timeout specified in passed argument > >> > >> Do you have any comments regarding this patch? > >> > > > > This test failed on machines with more than 40 cores: > > > > tst-select.c:54: 1616610088.851713938s not after 1616610089.851712804s > > (difference 0.999998866s) > > error: 1 test failures > > > > I was doing 3 "makec -j28 check" in parallel. > > I think the nanosecond precision of time accounting is triggering the > failure, since select only support timeval (the error indicates that > the nanosecond precision is what is triggering it). > > Maybe if we ignore the nanosecond precision: > > diff --git a/misc/tst-select.c b/misc/tst-select.c > index 7c310256c5..4b1791ac8a 100644 > --- a/misc/tst-select.c > +++ b/misc/tst-select.c > @@ -45,6 +45,8 @@ test_select_timeout (bool zero_tmo) > to.tv_sec = timeout; > ts = xclock_now (CLOCK_REALTIME); > ts = timespec_add (ts, (struct timespec) { timeout, 0 }); > + /* Ignore nanosecond precision since select only support microsecond. */ > + ts.tv_nsec = (ts.tv_nsec * 1000) / 1000; How does it work? It looks like a NOP to me. > /* Wait for timeout. */ > int ret = select (fds + 1, &rfds, NULL, NULL, &to);
On 24/03/2021 17:13, H.J. Lu wrote: > On Wed, Mar 24, 2021 at 12:47 PM Adhemerval Zanella > <adhemerval.zanella@linaro.org> wrote: >> >> >> >> On 24/03/2021 16:17, H.J. Lu wrote: >>> On Mon, Mar 22, 2021 at 4:33 AM Lukasz Majewski <lukma@denx.de> wrote: >>>> >>>> Dear Community, >>>> >>>>> This change adds new test to assess select()'s timeout related >>>>> functionality (the rdfs set provides valid fd - stderr - but during >>>>> normal program operation there is no data to be read, so one just >>>>> waits for timeout). >>>>> >>>>> To be more specific - two use cases are checked: >>>>> - if select() times out immediately when passed struct timeval has >>>>> zero values of tv_usec and tv_sec. >>>>> - if select() times out after timeout specified in passed argument >>>> >>>> Do you have any comments regarding this patch? >>>> >>> >>> This test failed on machines with more than 40 cores: >>> >>> tst-select.c:54: 1616610088.851713938s not after 1616610089.851712804s >>> (difference 0.999998866s) >>> error: 1 test failures >>> >>> I was doing 3 "makec -j28 check" in parallel. >> >> I think the nanosecond precision of time accounting is triggering the >> failure, since select only support timeval (the error indicates that >> the nanosecond precision is what is triggering it). >> >> Maybe if we ignore the nanosecond precision: >> >> diff --git a/misc/tst-select.c b/misc/tst-select.c >> index 7c310256c5..4b1791ac8a 100644 >> --- a/misc/tst-select.c >> +++ b/misc/tst-select.c >> @@ -45,6 +45,8 @@ test_select_timeout (bool zero_tmo) >> to.tv_sec = timeout; >> ts = xclock_now (CLOCK_REALTIME); >> ts = timespec_add (ts, (struct timespec) { timeout, 0 }); >> + /* Ignore nanosecond precision since select only support microsecond. */ >> + ts.tv_nsec = (ts.tv_nsec * 1000) / 1000; > > How does it work? It looks like a NOP to me. The idea is to clear the nanosecond precision from the xclock_now call.
On Wed, Mar 24, 2021 at 2:14 PM Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > > > On 24/03/2021 17:13, H.J. Lu wrote: > > On Wed, Mar 24, 2021 at 12:47 PM Adhemerval Zanella > > <adhemerval.zanella@linaro.org> wrote: > >> > >> > >> > >> On 24/03/2021 16:17, H.J. Lu wrote: > >>> On Mon, Mar 22, 2021 at 4:33 AM Lukasz Majewski <lukma@denx.de> wrote: > >>>> > >>>> Dear Community, > >>>> > >>>>> This change adds new test to assess select()'s timeout related > >>>>> functionality (the rdfs set provides valid fd - stderr - but during > >>>>> normal program operation there is no data to be read, so one just > >>>>> waits for timeout). > >>>>> > >>>>> To be more specific - two use cases are checked: > >>>>> - if select() times out immediately when passed struct timeval has > >>>>> zero values of tv_usec and tv_sec. > >>>>> - if select() times out after timeout specified in passed argument > >>>> > >>>> Do you have any comments regarding this patch? > >>>> > >>> > >>> This test failed on machines with more than 40 cores: > >>> > >>> tst-select.c:54: 1616610088.851713938s not after 1616610089.851712804s > >>> (difference 0.999998866s) > >>> error: 1 test failures > >>> > >>> I was doing 3 "makec -j28 check" in parallel. > >> > >> I think the nanosecond precision of time accounting is triggering the > >> failure, since select only support timeval (the error indicates that > >> the nanosecond precision is what is triggering it). > >> > >> Maybe if we ignore the nanosecond precision: > >> > >> diff --git a/misc/tst-select.c b/misc/tst-select.c > >> index 7c310256c5..4b1791ac8a 100644 > >> --- a/misc/tst-select.c > >> +++ b/misc/tst-select.c > >> @@ -45,6 +45,8 @@ test_select_timeout (bool zero_tmo) > >> to.tv_sec = timeout; > >> ts = xclock_now (CLOCK_REALTIME); > >> ts = timespec_add (ts, (struct timespec) { timeout, 0 }); > >> + /* Ignore nanosecond precision since select only support microsecond. */ > >> + ts.tv_nsec = (ts.tv_nsec * 1000) / 1000; > > > > How does it work? It looks like a NOP to me. > > The idea is to clear the nanosecond precision from the xclock_now call. I tried it. It doesn't solve the problem. I opened: https://sourceware.org/bugzilla/show_bug.cgi?id=27648
On 24/03/2021 18:21, H.J. Lu wrote: > On Wed, Mar 24, 2021 at 2:14 PM Adhemerval Zanella > <adhemerval.zanella@linaro.org> wrote: >> >> >> >> On 24/03/2021 17:13, H.J. Lu wrote: >>> On Wed, Mar 24, 2021 at 12:47 PM Adhemerval Zanella >>> <adhemerval.zanella@linaro.org> wrote: >>>> >>>> >>>> >>>> On 24/03/2021 16:17, H.J. Lu wrote: >>>>> On Mon, Mar 22, 2021 at 4:33 AM Lukasz Majewski <lukma@denx.de> wrote: >>>>>> >>>>>> Dear Community, >>>>>> >>>>>>> This change adds new test to assess select()'s timeout related >>>>>>> functionality (the rdfs set provides valid fd - stderr - but during >>>>>>> normal program operation there is no data to be read, so one just >>>>>>> waits for timeout). >>>>>>> >>>>>>> To be more specific - two use cases are checked: >>>>>>> - if select() times out immediately when passed struct timeval has >>>>>>> zero values of tv_usec and tv_sec. >>>>>>> - if select() times out after timeout specified in passed argument >>>>>> >>>>>> Do you have any comments regarding this patch? >>>>>> >>>>> >>>>> This test failed on machines with more than 40 cores: >>>>> >>>>> tst-select.c:54: 1616610088.851713938s not after 1616610089.851712804s >>>>> (difference 0.999998866s) >>>>> error: 1 test failures >>>>> >>>>> I was doing 3 "makec -j28 check" in parallel. >>>> >>>> I think the nanosecond precision of time accounting is triggering the >>>> failure, since select only support timeval (the error indicates that >>>> the nanosecond precision is what is triggering it). >>>> >>>> Maybe if we ignore the nanosecond precision: >>>> >>>> diff --git a/misc/tst-select.c b/misc/tst-select.c >>>> index 7c310256c5..4b1791ac8a 100644 >>>> --- a/misc/tst-select.c >>>> +++ b/misc/tst-select.c >>>> @@ -45,6 +45,8 @@ test_select_timeout (bool zero_tmo) >>>> to.tv_sec = timeout; >>>> ts = xclock_now (CLOCK_REALTIME); >>>> ts = timespec_add (ts, (struct timespec) { timeout, 0 }); >>>> + /* Ignore nanosecond precision since select only support microsecond. */ >>>> + ts.tv_nsec = (ts.tv_nsec * 1000) / 1000; >>> >>> How does it work? It looks like a NOP to me. >> >> The idea is to clear the nanosecond precision from the xclock_now call. > > I tried it. It doesn't solve the problem. I opened: > > https://sourceware.org/bugzilla/show_bug.cgi?id=27648 I think we will need to clear out on the TEST_TIMESPEC_NOW_OR_AFTER as well (by adding another macro).
H.J. Lu via Libc-alpha, le mer. 24 mars 2021 14:21:54 -0700, a ecrit: > On Wed, Mar 24, 2021 at 2:14 PM Adhemerval Zanella > <adhemerval.zanella@linaro.org> wrote: > > > > > > > > On 24/03/2021 17:13, H.J. Lu wrote: > > > On Wed, Mar 24, 2021 at 12:47 PM Adhemerval Zanella > > > <adhemerval.zanella@linaro.org> wrote: > > >> > > >> > > >> > > >> On 24/03/2021 16:17, H.J. Lu wrote: > > >>> On Mon, Mar 22, 2021 at 4:33 AM Lukasz Majewski <lukma@denx.de> wrote: > > >>>> > > >>>> Dear Community, > > >>>> > > >>>>> This change adds new test to assess select()'s timeout related > > >>>>> functionality (the rdfs set provides valid fd - stderr - but during > > >>>>> normal program operation there is no data to be read, so one just > > >>>>> waits for timeout). > > >>>>> > > >>>>> To be more specific - two use cases are checked: > > >>>>> - if select() times out immediately when passed struct timeval has > > >>>>> zero values of tv_usec and tv_sec. > > >>>>> - if select() times out after timeout specified in passed argument > > >>>> > > >>>> Do you have any comments regarding this patch? > > >>>> > > >>> > > >>> This test failed on machines with more than 40 cores: > > >>> > > >>> tst-select.c:54: 1616610088.851713938s not after 1616610089.851712804s > > >>> (difference 0.999998866s) > > >>> error: 1 test failures > > >>> > > >>> I was doing 3 "makec -j28 check" in parallel. > > >> > > >> I think the nanosecond precision of time accounting is triggering the > > >> failure, since select only support timeval (the error indicates that > > >> the nanosecond precision is what is triggering it). > > >> > > >> Maybe if we ignore the nanosecond precision: > > >> > > >> diff --git a/misc/tst-select.c b/misc/tst-select.c > > >> index 7c310256c5..4b1791ac8a 100644 > > >> --- a/misc/tst-select.c > > >> +++ b/misc/tst-select.c > > >> @@ -45,6 +45,8 @@ test_select_timeout (bool zero_tmo) > > >> to.tv_sec = timeout; > > >> ts = xclock_now (CLOCK_REALTIME); > > >> ts = timespec_add (ts, (struct timespec) { timeout, 0 }); > > >> + /* Ignore nanosecond precision since select only support microsecond. */ > > >> + ts.tv_nsec = (ts.tv_nsec * 1000) / 1000; > > > > > > How does it work? It looks like a NOP to me. > > > > The idea is to clear the nanosecond precision from the xclock_now call. It should rather be ts.tv_nsec = (ts.tv_nsec / 1000) * 1000; shouldn't it? Samuel
On 24/03/2021 18:35, Samuel Thibault wrote: > H.J. Lu via Libc-alpha, le mer. 24 mars 2021 14:21:54 -0700, a ecrit: >> On Wed, Mar 24, 2021 at 2:14 PM Adhemerval Zanella >> <adhemerval.zanella@linaro.org> wrote: >>> >>> >>> >>> On 24/03/2021 17:13, H.J. Lu wrote: >>>> On Wed, Mar 24, 2021 at 12:47 PM Adhemerval Zanella >>>> <adhemerval.zanella@linaro.org> wrote: >>>>> >>>>> >>>>> >>>>> On 24/03/2021 16:17, H.J. Lu wrote: >>>>>> On Mon, Mar 22, 2021 at 4:33 AM Lukasz Majewski <lukma@denx.de> wrote: >>>>>>> >>>>>>> Dear Community, >>>>>>> >>>>>>>> This change adds new test to assess select()'s timeout related >>>>>>>> functionality (the rdfs set provides valid fd - stderr - but during >>>>>>>> normal program operation there is no data to be read, so one just >>>>>>>> waits for timeout). >>>>>>>> >>>>>>>> To be more specific - two use cases are checked: >>>>>>>> - if select() times out immediately when passed struct timeval has >>>>>>>> zero values of tv_usec and tv_sec. >>>>>>>> - if select() times out after timeout specified in passed argument >>>>>>> >>>>>>> Do you have any comments regarding this patch? >>>>>>> >>>>>> >>>>>> This test failed on machines with more than 40 cores: >>>>>> >>>>>> tst-select.c:54: 1616610088.851713938s not after 1616610089.851712804s >>>>>> (difference 0.999998866s) >>>>>> error: 1 test failures >>>>>> >>>>>> I was doing 3 "makec -j28 check" in parallel. >>>>> >>>>> I think the nanosecond precision of time accounting is triggering the >>>>> failure, since select only support timeval (the error indicates that >>>>> the nanosecond precision is what is triggering it). >>>>> >>>>> Maybe if we ignore the nanosecond precision: >>>>> >>>>> diff --git a/misc/tst-select.c b/misc/tst-select.c >>>>> index 7c310256c5..4b1791ac8a 100644 >>>>> --- a/misc/tst-select.c >>>>> +++ b/misc/tst-select.c >>>>> @@ -45,6 +45,8 @@ test_select_timeout (bool zero_tmo) >>>>> to.tv_sec = timeout; >>>>> ts = xclock_now (CLOCK_REALTIME); >>>>> ts = timespec_add (ts, (struct timespec) { timeout, 0 }); >>>>> + /* Ignore nanosecond precision since select only support microsecond. */ >>>>> + ts.tv_nsec = (ts.tv_nsec * 1000) / 1000; >>>> >>>> How does it work? It looks like a NOP to me. >>> >>> The idea is to clear the nanosecond precision from the xclock_now call. > > It should rather be > > ts.tv_nsec = (ts.tv_nsec / 1000) * 1000; > > shouldn't it? Indeed, thanks for spotting it. Could you check if this helps the tests H.J?
On Wed, Mar 24, 2021 at 2:43 PM Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > > > On 24/03/2021 18:35, Samuel Thibault wrote: > > H.J. Lu via Libc-alpha, le mer. 24 mars 2021 14:21:54 -0700, a ecrit: > >> On Wed, Mar 24, 2021 at 2:14 PM Adhemerval Zanella > >> <adhemerval.zanella@linaro.org> wrote: > >>> > >>> > >>> > >>> On 24/03/2021 17:13, H.J. Lu wrote: > >>>> On Wed, Mar 24, 2021 at 12:47 PM Adhemerval Zanella > >>>> <adhemerval.zanella@linaro.org> wrote: > >>>>> > >>>>> > >>>>> > >>>>> On 24/03/2021 16:17, H.J. Lu wrote: > >>>>>> On Mon, Mar 22, 2021 at 4:33 AM Lukasz Majewski <lukma@denx.de> wrote: > >>>>>>> > >>>>>>> Dear Community, > >>>>>>> > >>>>>>>> This change adds new test to assess select()'s timeout related > >>>>>>>> functionality (the rdfs set provides valid fd - stderr - but during > >>>>>>>> normal program operation there is no data to be read, so one just > >>>>>>>> waits for timeout). > >>>>>>>> > >>>>>>>> To be more specific - two use cases are checked: > >>>>>>>> - if select() times out immediately when passed struct timeval has > >>>>>>>> zero values of tv_usec and tv_sec. > >>>>>>>> - if select() times out after timeout specified in passed argument > >>>>>>> > >>>>>>> Do you have any comments regarding this patch? > >>>>>>> > >>>>>> > >>>>>> This test failed on machines with more than 40 cores: > >>>>>> > >>>>>> tst-select.c:54: 1616610088.851713938s not after 1616610089.851712804s > >>>>>> (difference 0.999998866s) > >>>>>> error: 1 test failures > >>>>>> > >>>>>> I was doing 3 "makec -j28 check" in parallel. > >>>>> > >>>>> I think the nanosecond precision of time accounting is triggering the > >>>>> failure, since select only support timeval (the error indicates that > >>>>> the nanosecond precision is what is triggering it). > >>>>> > >>>>> Maybe if we ignore the nanosecond precision: > >>>>> > >>>>> diff --git a/misc/tst-select.c b/misc/tst-select.c > >>>>> index 7c310256c5..4b1791ac8a 100644 > >>>>> --- a/misc/tst-select.c > >>>>> +++ b/misc/tst-select.c > >>>>> @@ -45,6 +45,8 @@ test_select_timeout (bool zero_tmo) > >>>>> to.tv_sec = timeout; > >>>>> ts = xclock_now (CLOCK_REALTIME); > >>>>> ts = timespec_add (ts, (struct timespec) { timeout, 0 }); > >>>>> + /* Ignore nanosecond precision since select only support microsecond. */ > >>>>> + ts.tv_nsec = (ts.tv_nsec * 1000) / 1000; > >>>> > >>>> How does it work? It looks like a NOP to me. > >>> > >>> The idea is to clear the nanosecond precision from the xclock_now call. > > > > It should rather be > > > > ts.tv_nsec = (ts.tv_nsec / 1000) * 1000; > > > > shouldn't it? > > Indeed, thanks for spotting it. Could you check if this helps the tests H.J? The test failed with "nohup make check" since select returned immediately.
On 24/03/2021 19:30, H.J. Lu wrote: >>>>>> How does it work? It looks like a NOP to me. >>>>> >>>>> The idea is to clear the nanosecond precision from the xclock_now call. >>> >>> It should rather be >>> >>> ts.tv_nsec = (ts.tv_nsec / 1000) * 1000; >>> >>> shouldn't it? >> >> Indeed, thanks for spotting it. Could you check if this helps the tests H.J? > > The test failed with "nohup make check" since select returned immediately. > Do you have a strace of why select is returning? It uses 2 (STDERR_FILENO) as on the read set, so it should trigger the timeout.
On Mär 25 2021, Adhemerval Zanella via Libc-alpha wrote: > Do you have a strace of why select is returning? It uses 2 (STDERR_FILENO) > as on the read set, so it should trigger the timeout. A file is always available, so there is no timeout. $ ./testrun.sh misc/tst-select 2>nohup.out tst-select.c:54: 1616674766.364168922s not after 1616674767.364165762s (difference 0.999996840s) error: 1 test failures Andreas.
On 25/03/2021 09:20, Andreas Schwab wrote: > On Mär 25 2021, Adhemerval Zanella via Libc-alpha wrote: > >> Do you have a strace of why select is returning? It uses 2 (STDERR_FILENO) >> as on the read set, so it should trigger the timeout. > > A file is always available, so there is no timeout. > > $ ./testrun.sh misc/tst-select 2>nohup.out > tst-select.c:54: 1616674766.364168922s not after 1616674767.364165762s (difference 0.999996840s) > error: 1 test failures I did not catch it on my review, we will need a temporary file for this. I will check this out.
On Thu, Mar 25, 2021 at 5:48 AM Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > > > On 25/03/2021 09:20, Andreas Schwab wrote: > > On Mär 25 2021, Adhemerval Zanella via Libc-alpha wrote: > > > >> Do you have a strace of why select is returning? It uses 2 (STDERR_FILENO) > >> as on the read set, so it should trigger the timeout. > > > > A file is always available, so there is no timeout. > > > > $ ./testrun.sh misc/tst-select 2>nohup.out > > tst-select.c:54: 1616674766.364168922s not after 1616674767.364165762s (difference 0.999996840s) > > error: 1 test failures > > I did not catch it on my review, we will need a temporary file for this. > I will check this out. > Can we do something similar to misc/tst-pselect.c?
On 25/03/2021 10:22, H.J. Lu wrote: > On Thu, Mar 25, 2021 at 5:48 AM Adhemerval Zanella > <adhemerval.zanella@linaro.org> wrote: >> >> >> >> On 25/03/2021 09:20, Andreas Schwab wrote: >>> On Mär 25 2021, Adhemerval Zanella via Libc-alpha wrote: >>> >>>> Do you have a strace of why select is returning? It uses 2 (STDERR_FILENO) >>>> as on the read set, so it should trigger the timeout. >>> >>> A file is always available, so there is no timeout. >>> >>> $ ./testrun.sh misc/tst-select 2>nohup.out >>> tst-select.c:54: 1616674766.364168922s not after 1616674767.364165762s (difference 0.999996840s) >>> error: 1 test failures >> >> I did not catch it on my review, we will need a temporary file for this. >> I will check this out. >> > > Can we do something similar to misc/tst-pselect.c? > Yeah, I think it would be better. I will prepare a patch.
diff --git a/misc/Makefile b/misc/Makefile index a8363d4b76..a3545f047e 100644 --- a/misc/Makefile +++ b/misc/Makefile @@ -88,7 +88,7 @@ tests := tst-dirname tst-tsearch tst-fdset tst-mntent tst-hsearch \ tst-preadvwritev tst-preadvwritev64 tst-makedev tst-empty \ tst-preadvwritev2 tst-preadvwritev64v2 tst-warn-wide \ tst-ldbl-warn tst-ldbl-error tst-dbl-efgcvt tst-ldbl-efgcvt \ - tst-mntent-autofs tst-syscalls tst-mntent-escape + tst-mntent-autofs tst-syscalls tst-mntent-escape tst-select tests-time64 := tst-pselect-time64 diff --git a/misc/tst-select.c b/misc/tst-select.c new file mode 100644 index 0000000000..ea776c8880 --- /dev/null +++ b/misc/tst-select.c @@ -0,0 +1,70 @@ +/* Test for select timeout + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <time.h> +#include <errno.h> +#include <stdbool.h> +#include <sys/select.h> +#include <support/check.h> +#include <support/xtime.h> +#include <support/timespec.h> + +#define TST_SELECT_TIMEOUT 1 +#define TST_SELECT_FD_ERR 2 + +static int test_select_timeout (bool zero_tmo) +{ + const int fds = TST_SELECT_FD_ERR; + int timeout = TST_SELECT_TIMEOUT; + struct timeval to = { 0, 0 }; + struct timespec ts; + fd_set rfds; + + FD_ZERO (&rfds); + FD_SET (fds, &rfds); + + if (zero_tmo) + timeout = 0; + + to.tv_sec = timeout; + ts = xclock_now (CLOCK_REALTIME); + ts = timespec_add (ts, (struct timespec) { timeout, 0 }); + + /* Wait for timeout. */ + int ret = select (fds + 1, &rfds, NULL, NULL, &to); + if (ret == -1) + FAIL_EXIT1 ("select failed: %m\n"); + + TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts); + + return 0; +} + +static int +do_test (void) +{ + /* Check if select exits immediately. */ + test_select_timeout (true); + + /* Check if select exits after specified timeout. */ + test_select_timeout (false); + + return 0; +} + +#include <support/test-driver.c>