Re: [PATCH v2] tst: Provide test for ppoll

[Lukasz Majewski <lukma@denx.de>]

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

Hi Adhemerval,

> On 14/01/2021 13:32, Lukasz Majewski wrote:
> > This change adds new test to assess ppoll()'s timeout related
> > functionality (the struct pollfd does not provide valid fd to wait
> > for - just wait for timeout).
> > 
> > To be more specific - two use cases are checked:
> > - if ppoll() times out immediately when passed struct timespec has
> > zero values of tv_nsec and tv_sec.
> > - if ppoll() times out after timeout specified in passed argument
> > 
> > ---
> > Changes for v2:
> > - Remove _GNU_SOURCE definition if not already defined
> > - Replace clock_gettime with xclock_gettime
> > - Use FAIL_EXIT1 instead of plain ret value returning from the test
> > ---
> >  sysdeps/unix/sysv/linux/Makefile    |  2 +-
> >  sysdeps/unix/sysv/linux/tst-ppoll.c | 62
> > +++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1
> > deletion(-) create mode 100644 sysdeps/unix/sysv/linux/tst-ppoll.c
> > 
> > diff --git a/sysdeps/unix/sysv/linux/Makefile
> > b/sysdeps/unix/sysv/linux/Makefile index 7503b356c8..f4029a74ca
> > 100644 --- a/sysdeps/unix/sysv/linux/Makefile
> > +++ b/sysdeps/unix/sysv/linux/Makefile
> > @@ -108,7 +108,7 @@ tests += tst-clone tst-clone2 tst-clone3
> > tst-fanotify tst-personality \ test-errno-linux tst-memfd_create
> > tst-mlock2 tst-pkey \ tst-rlimit-infinity tst-ofdlocks tst-gettid
> > tst-gettid-kill \ tst-tgkill tst-sysvsem-linux tst-sysvmsg-linux
> > tst-sysvshm-linux \
> > -	 tst-timerfd
> > +	 tst-timerfd tst-ppoll
> >  tests-internal += tst-ofdlocks-compat tst-sigcontext-get_pc
> >  
> >  CFLAGS-tst-sigcontext-get_pc.c = -fasynchronous-unwind-tables  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/tst-ppoll.c
> > b/sysdeps/unix/sysv/linux/tst-ppoll.c new file mode 100644
> > index 0000000000..eeff97f103
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/tst-ppoll.c
> > @@ -0,0 +1,62 @@
> > +/* Test for ppoll 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 <poll.h>
> > +#include <support/check.h>
> > +#include <support/xtime.h>
> > +
> > +/* Timeout in seconds for PPOLL.  */
> > +#define PPOLL_TIMEOUT 2
> > +/* Timeout for test program - must be larger than PPOLL_TIMEOUT.
> > */ +#define TIMEOUT 3  
> 
> This define is unused. 

When TIMEOUT is not specified - the default time out for test is used
(2 seconds). For the ppoll test we want to wait for 2 seconds, so
without this local #define TIMEOUT 3 the test is terminated by glibc
test suite before we go out from the ppoll.

However, as we now wait for 0.5 sec, it is feasible to just remove it
and used default settings.

> 
> > +
> > +static int test_ppoll_timeout (int timeout)
> > +{
> > +  struct timespec tv0 = { 0, 0 }, tv1 = { 0, 0 }, tv = { 0, 0 };
> > +  struct pollfd fds = { -1, 0, 0 };   /* Ignore fds - just wait
> > for timeout.  */
> > +  int ret;
> > +
> > +  tv.tv_sec = timeout;
> > +
> > +  xclock_gettime (CLOCK_REALTIME, &tv0);
> > +
> > +  ret = ppoll (&fds, 1, &tv, 0);
> > +  if (ret != 0)
> > +      FAIL_EXIT1 ("*** ppoll failed: %m\n");
> > +
> > +  xclock_gettime (CLOCK_REALTIME, &tv1);
> > +  if (tv0.tv_sec + timeout != tv1.tv_sec)
> > +      FAIL_EXIT1 ("*** ppoll failed to timeout after %d [s]\n",
> > timeout); +
> > +  return 0;
> > +}
> > +
> > +static int
> > +do_test (void)
> > +{
> > +  /* Check if ppoll exits immediately.  */
> > +  test_ppoll_timeout (0);
> > +
> > +  /* Check if ppoll exits after specified timeout.  */
> > +  test_ppoll_timeout (PPOLL_TIMEOUT);
> > +
> > +  return 0;
> > +}
> > +
> > +#include <support/test-driver.c>  
> 
> The exactly timeout check is subjected to a scheduler pressure and it
> might indicate false positive depending of the system load.  It would
> be better to check only if subsequent clock_gettime returns a time
> after previously than the ppoll check.  Like:
> 
> static int test_ppoll_timeout (bool zero_tmo)
> { 
>   /* We wait for half a second.  */
>   struct timespec ts;
>   xclock_gettime (CLOCK_REALTIME, &ts);
>   struct timespec timeout = make_timespec (0, zero_tmo ? 0 :
> TIMESPEC_HZ/2); ts = timespec_add (ts, timeout);
> 
>   /* Ignore fds - just wait for timeout.  */
>   struct pollfd fds = { -1, 0, 0 };
>   TEST_COMPARE (ppoll (&fds, 1, &timeout, 0), 0);
> 
>   TEST_TIMESPEC_NOW_OR_AFTER (CLOCK_REALTIME, ts);
> 
>   return 0;
> }
> 
> static int
> do_test (void)
> { 
>   /* Check if ppoll exits immediately.  */
>   test_ppoll_timeout (true);
> 
>   /* Check if ppoll exits after specified timeout.  */
>   test_ppoll_timeout (false);
> 
>   return 0;
> }
> 
> 

Thanks for the tip. I will add this code to the test.


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