[PATCH] tst: Provide test for select

[PATCH] tst: Provide test for select

[Lukasz Majewski]

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

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>
-- 
2.20.1

Re: [PATCH] tst: Provide test for select

[DJ Delorie]

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

> 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>

Re: [PATCH] tst: Provide test for select

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 1331 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] tst: Provide test for select

[DJ Delorie]

Lukasz Majewski <lukma@denx.de> writes:
> And I do prefer to CC directly people who are involved - so they are
> aware of the work status.

We're on the mailing list, we are aware.  Please follow the documented
procedures.

Re: [PATCH] tst: Provide test for select

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 3917 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] tst: Provide test for select

[Adhemerval Zanella]

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.

Re: [PATCH] tst: Provide test for select

[H.J. Lu]

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.

Re: [PATCH] tst: Provide test for select

[Adhemerval Zanella]

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);

Re: [PATCH] tst: Provide test for select

[H.J. Lu]

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);



-- 
H.J.

Re: [PATCH] tst: Provide test for select

[Adhemerval Zanella]

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.

Re: [PATCH] tst: Provide test for select

[H.J. Lu]

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

-- 
H.J.

Re: [PATCH] tst: Provide test for select

[Adhemerval Zanella]

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).

Re: [PATCH] tst: Provide test for select

[Samuel Thibault]

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

Re: [PATCH] tst: Provide test for select

[Adhemerval Zanella]

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?

Re: [PATCH] tst: Provide test for select

[H.J. Lu]

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.

-- 
H.J.

Re: [PATCH] tst: Provide test for select

[Adhemerval Zanella]

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.

Re: [PATCH] tst: Provide test for select

[Andreas Schwab]

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.

-- 
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."

Re: [PATCH] tst: Provide test for select

[Adhemerval Zanella]

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.

Re: [PATCH] tst: Provide test for select

[H.J. Lu]

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?

-- 
H.J.

Re: [PATCH] tst: Provide test for select

[Adhemerval Zanella]

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.