[LTP] [RFC PATCH 1/2] tst_test: Add test multiplex function

[LTP] [RFC PATCH 1/2] tst_test: Add test multiplex function

[Cyril Hrubis]

From: Cyril Hrubis <chrubis@suse.cz>

The test multiplex function is intended for running the test function in
a loop for a different settings. The indended purpose is to run tests
for both libc wrapper and raw syscall as well as for different syscall
variants.

The commit itself adds a test_multiplex() function into the tst_test
structure, if set this function is called in a loop before each test
iteration and is responsible for changing the test variant into next
one. The iterations continue until the function returns zero.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
CC: Mark Salyzyn <salyzyn@android.com>
CC: Steve Muckle <smuckle@google.com>
CC: Jan Stancek <jstancek@redhat.com>
---
 include/tst_test.h | 14 ++++++++++++++
 lib/tst_test.c     | 11 +++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/include/tst_test.h b/include/tst_test.h
index da41f776d..b747b24d5 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -146,6 +146,20 @@ struct tst_test {
 	 */
 	int all_filesystems:1;
 
+	/*
+	 * If set this function is used to mutiplex between different test
+	 * variants.
+	 *
+	 * Multiplexers are the way how to run the same test for different
+	 * settings. The intended use is to test different syscall
+	 * wrappers/variants but the API is generic and does not limit the
+	 * usage in any way.
+	 *
+	 * Each time the function is called it switches to next test variant,
+	 * returns zero if all variants has been iterated over.
+	 */
+	int (*test_multiplex)(void);
+
 	/* Minimal device size in megabytes */
 	unsigned int dev_min_size;
 
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 7dd890b8d..6b033c879 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1009,8 +1009,15 @@ static void testrun(void)
 		if (!cont)
 			break;
 
-		run_tests();
-		heartbeat();
+		if (tst_test->test_multiplex) {
+			while (tst_test->test_multiplex()) {
+				run_tests();
+				heartbeat();
+			}
+		} else {
+			run_tests();
+			heartbeat();
+		}
 	}
 
 	do_test_cleanup();
-- 
2.19.2

[LTP] [RFC PATCH 1/2] tst_test: Add test multiplex function

[Cyril Hrubis]

Hi!
Sigh, misconfiguration here, should have been send from my SUSE account,
please ignore this one.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [RFC PATCH 1/2] tst_test: Add test multiplex function

[Cyril Hrubis]

The test multiplex function is intended for running the test function in
a loop for a different settings. The indended purpose is to run tests
for both libc wrapper and raw syscall as well as for different syscall
variants.

The commit itself adds a test_multiplex() function into the tst_test
structure, if set this function is called in a loop before each test
iteration and is responsible for changing the test variant into next
one. The iterations continue until the function returns zero.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
CC: Mark Salyzyn <salyzyn@android.com>
CC: Steve Muckle <smuckle@google.com>
CC: Jan Stancek <jstancek@redhat.com>
---
 include/tst_test.h | 14 ++++++++++++++
 lib/tst_test.c     | 11 +++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/include/tst_test.h b/include/tst_test.h
index da41f776d..b747b24d5 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -146,6 +146,20 @@ struct tst_test {
 	 */
 	int all_filesystems:1;
 
+	/*
+	 * If set this function is used to mutiplex between different test
+	 * variants.
+	 *
+	 * Multiplexers are the way how to run the same test for different
+	 * settings. The intended use is to test different syscall
+	 * wrappers/variants but the API is generic and does not limit the
+	 * usage in any way.
+	 *
+	 * Each time the function is called it switches to next test variant,
+	 * returns zero if all variants has been iterated over.
+	 */
+	int (*test_multiplex)(void);
+
 	/* Minimal device size in megabytes */
 	unsigned int dev_min_size;
 
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 7dd890b8d..6b033c879 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1009,8 +1009,15 @@ static void testrun(void)
 		if (!cont)
 			break;
 
-		run_tests();
-		heartbeat();
+		if (tst_test->test_multiplex) {
+			while (tst_test->test_multiplex()) {
+				run_tests();
+				heartbeat();
+			}
+		} else {
+			run_tests();
+			heartbeat();
+		}
 	}
 
 	do_test_cleanup();
-- 
2.19.2

[LTP] [RFC PATCH 2/2] syscalls/select04: Test four syscall variants

[Cyril Hrubis]

This commit makes use of the newly added test_multiplex() function to
switch between different syscall variants/wrappers at runtime.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
CC: Mark Salyzyn <salyzyn@android.com>
CC: Steve Muckle <smuckle@google.com>
CC: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/select/select04.c   |  5 +-
 testcases/kernel/syscalls/select/select_mpx.h | 79 +++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 testcases/kernel/syscalls/select/select_mpx.h

diff --git a/testcases/kernel/syscalls/select/select04.c b/testcases/kernel/syscalls/select/select04.c
index 86bdffcdf..300992233 100644
--- a/testcases/kernel/syscalls/select/select04.c
+++ b/testcases/kernel/syscalls/select/select04.c
@@ -27,6 +27,8 @@
 
 #include "tst_timer_test.h"
 
+#include "select_mpx.h"
+
 static int fds[2];
 
 static int sample_fn(int clk_id, long long usec)
@@ -39,7 +41,7 @@ static int sample_fn(int clk_id, long long usec)
 	FD_SET(fds[0], &sfds);
 
 	tst_timer_start(clk_id);
-	TEST(select(1, &sfds, NULL, NULL, &timeout));
+	TEST(do_select(1, &sfds, NULL, NULL, &timeout));
 	tst_timer_stop();
 	tst_timer_sample();
 
@@ -69,5 +71,6 @@ static struct tst_test test = {
 	.scall = "select()",
 	.sample = sample_fn,
 	.setup = setup,
+	.test_multiplex = select_mpx,
 	.cleanup = cleanup,
 };
diff --git a/testcases/kernel/syscalls/select/select_mpx.h b/testcases/kernel/syscalls/select/select_mpx.h
new file mode 100644
index 000000000..39c4e6517
--- /dev/null
+++ b/testcases/kernel/syscalls/select/select_mpx.h
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2019 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+#ifndef SELECT_MPX
+#define SELECT_MPX
+
+#include "lapi/syscalls.h"
+
+static int sys_mpx = -1;
+
+struct compat_sel_arg_struct {
+	long _n;
+	long _inp;
+	long _outp;
+	long _exp;
+	long _tvp;
+};
+
+static int do_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, struct timeval *timeout)
+{
+	switch (sys_mpx) {
+	case 0:
+		return select(nfds, readfds, writefds, exceptfds, timeout);
+	break;
+	case 1:
+		return tst_syscall(__NR__newselect, nfds, readfds, writefds, exceptfds, timeout);
+	break;
+	case 2: {
+		struct compat_sel_arg_struct arg = {
+			._n = (long)nfds,
+			._inp = (long)readfds,
+			._outp = (long)writefds,
+			._exp = (long)exceptfds,
+			._tvp = (long)timeout,
+		};
+
+		return tst_syscall(__NR_select, &arg);
+	}
+	case 3: {
+		int ret;
+		struct timespec ts = {
+			.tv_sec = timeout->tv_sec,
+			.tv_nsec = timeout->tv_usec * 1000,
+		};
+		ret = tst_syscall(__NR_pselect6, nfds, readfds, writefds, exceptfds, &ts, NULL);
+		timeout->tv_sec = ts.tv_sec;
+		timeout->tv_usec = ts.tv_nsec / 1000;
+		return ret;
+	}
+	}
+
+	return -1;
+}
+
+static int select_mpx(void)
+{
+	switch (++sys_mpx) {
+	case 0:
+		tst_res(TINFO, "Testing glibc select()");
+	break;
+	case 1:
+		tst_res(TINFO, "Testing SYS__newselect syscall");
+	break;
+	case 2:
+		tst_res(TINFO, "Testing SYS_select syscall");
+	break;
+	case 3:
+		tst_res(TINFO, "Testing SYS_pselect6 syscall");
+	break;
+	case 4:
+		sys_mpx = -1;
+		return 0;
+	}
+
+	return 1;
+}
+
+#endif /* SELECT_MPX */
-- 
2.19.2

[LTP] [RFC PATCH 2/2] syscalls/select04: Test four syscall variants

[Mark Salyzyn]

_THANKS_ for the set of changes!

On 03/06/2019 07:24 AM, Cyril Hrubis wrote:
> +	case 2: {
> +		struct compat_sel_arg_struct arg = {
> +			._n = (long)nfds,
> +			._inp = (long)readfds,
> +			._outp = (long)writefds,
> +			._exp = (long)exceptfds,
> +			._tvp = (long)timeout,
> +		};
> +
> +		return tst_syscall(__NR_select, &arg);
> +	}

This fragment is correct for 32-bit configurations and 64-bit compat, 
but for 64-bit binaries it is:

#if defined(__LP64__)
return tst_syscall(__NR_select, nfds, readfds, writefds, exceptfds, timeout);
#else
. . .
#endif

-- Mark

[LTP] [RFC PATCH 1/2] tst_test: Add test multiplex function

[Jan Stancek]

----- Original Message -----
> The test multiplex function is intended for running the test function in
> a loop for a different settings. The indended purpose is to run tests
> for both libc wrapper and raw syscall as well as for different syscall
> variants.
> 
> The commit itself adds a test_multiplex() function into the tst_test
> structure, if set this function is called in a loop before each test
> iteration and is responsible for changing the test variant into next
> one. The iterations continue until the function returns zero.

Hi,

on first look this looks like a workaround, because we have locked
ourselves out of .test function for timer tests.

Is there some way we could allow .test again for times tests?
Something user could override, provided he then runs the library
function that iterates over all usec values.

(sorry for pseudo-code)
// test
void my_test(int n)
{
  select_func = n;
  test_timer_test();
}

// library
void test_timer_test()
{
  for (i = 0; i < ARRAY_SIZE(tcases); i++)
    timer_test_fn(i);
}

Regards,
Jan

> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> CC: Mark Salyzyn <salyzyn@android.com>
> CC: Steve Muckle <smuckle@google.com>
> CC: Jan Stancek <jstancek@redhat.com>
> ---
>  include/tst_test.h | 14 ++++++++++++++
>  lib/tst_test.c     | 11 +++++++++--
>  2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/include/tst_test.h b/include/tst_test.h
> index da41f776d..b747b24d5 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -146,6 +146,20 @@ struct tst_test {
>  	 */
>  	int all_filesystems:1;
>  
> +	/*
> +	 * If set this function is used to mutiplex between different test
> +	 * variants.
> +	 *
> +	 * Multiplexers are the way how to run the same test for different
> +	 * settings. The intended use is to test different syscall
> +	 * wrappers/variants but the API is generic and does not limit the
> +	 * usage in any way.
> +	 *
> +	 * Each time the function is called it switches to next test variant,
> +	 * returns zero if all variants has been iterated over.
> +	 */
> +	int (*test_multiplex)(void);
> +
>  	/* Minimal device size in megabytes */
>  	unsigned int dev_min_size;
>  
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 7dd890b8d..6b033c879 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1009,8 +1009,15 @@ static void testrun(void)
>  		if (!cont)
>  			break;
>  
> -		run_tests();
> -		heartbeat();
> +		if (tst_test->test_multiplex) {
> +			while (tst_test->test_multiplex()) {
> +				run_tests();
> +				heartbeat();
> +			}
> +		} else {
> +			run_tests();
> +			heartbeat();
> +		}
>  	}
>  
>  	do_test_cleanup();
> --
> 2.19.2
> 
> 

[LTP] [RFC PATCH 1/2] tst_test: Add test multiplex function

[Cyril Hrubis]

Hi!
> on first look this looks like a workaround, because we have locked
> ourselves out of .test function for timer tests.

I do not follow you here, can you elaborate?

This patch has nothing to do with timer tests, it just allows whatever
the test does to be done several times with a hook to change some
settings prior to each iteration.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [RFC PATCH 1/2] tst_test: Add test multiplex function

[Jan Stancek]

----- Original Message -----
> Hi!
> > on first look this looks like a workaround, because we have locked
> > ourselves out of .test function for timer tests.
> 
> I do not follow you here, can you elaborate?

If this wasn't timer test, I'd ask why don't we use existing .test and .tcnt,
your test() func can be called with a parameter, so you could change
the code to choose correct syscall/glibc func based on value of that parameter.

For normal tests, this looks almost and .test/.tcnt functionality,
except test count can be also dynamic.

static int tcase = -1;

static void test(void)
{
   switch (tcase) {
   }
}

static int select_mpx(void)
{
    tcase++;
    if (tcase == 5)
      return 0;
    return 1;
}

static struct tst_test test = {
    .test_multiplex = select_mpx,
    .test_all = test,
}

> 
> This patch has nothing to do with timer tests, it just allows whatever
> the test does to be done several times with a hook to change some
> settings prior to each iteration.
> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

[LTP] [RFC PATCH 1/2] tst_test: Add test multiplex function

[Cyril Hrubis]

Hi!
> > > on first look this looks like a workaround, because we have locked
> > > ourselves out of .test function for timer tests.
> > 
> > I do not follow you here, can you elaborate?
> 
> If this wasn't timer test, I'd ask why don't we use existing .test and .tcnt,
> your test() func can be called with a parameter, so you could change
> the code to choose correct syscall/glibc func based on value of that parameter.
> 
> For normal tests, this looks almost and .test/.tcnt functionality,
> except test count can be also dynamic.

Well so does the .all_filesystems flag, however I see these concepts to
be orthogonal to what the actual test does, which is the reason I want
them hooked up in the library rather than to be part of the testcase
code.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [RFC PATCH 1/2] tst_test: Add test multiplex function

[Jan Stancek]

----- Original Message -----
> Hi!
> > > > on first look this looks like a workaround, because we have locked
> > > > ourselves out of .test function for timer tests.
> > > 
> > > I do not follow you here, can you elaborate?
> > 
> > If this wasn't timer test, I'd ask why don't we use existing .test and
> > .tcnt,
> > your test() func can be called with a parameter, so you could change
> > the code to choose correct syscall/glibc func based on value of that
> > parameter.
> > 
> > For normal tests, this looks almost and .test/.tcnt functionality,
> > except test count can be also dynamic.
> 
> Well so does the .all_filesystems flag, however I see these concepts to
> be orthogonal to what the actual test does, which is the reason I want
> them hooked up in the library rather than to be part of the testcase
> code.

OK, so it's like another level on top of current test functions.

I probably would like 'variant' somewhere in name more, but overall
I'm not against the patch. Are you planning on adding some docs too?

Regards,
Jan

[LTP] [RFC PATCH 1/2] tst_test: Add test multiplex function

[Steve Muckle]

On 03/06/2019 07:24 AM, Cyril Hrubis wrote:
> The test multiplex function is intended for running the test function in
> a loop for a different settings. The indended purpose is to run tests
> for both libc wrapper and raw syscall as well as for different syscall
> variants.
> 
> The commit itself adds a test_multiplex() function into the tst_test
> structure, if set this function is called in a loop before each test
> iteration and is responsible for changing the test variant into next
> one. The iterations continue until the function returns zero.
> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> CC: Mark Salyzyn <salyzyn@android.com>
> CC: Steve Muckle <smuckle@google.com>
> CC: Jan Stancek <jstancek@redhat.com>

Approach seems good to me.

Reviewed-by: Steve Muckle <smuckle@google.com>

> ---
>   include/tst_test.h | 14 ++++++++++++++
>   lib/tst_test.c     | 11 +++++++++--
>   2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/include/tst_test.h b/include/tst_test.h
> index da41f776d..b747b24d5 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -146,6 +146,20 @@ struct tst_test {
>   	 */
>   	int all_filesystems:1;
>   
> +	/*
> +	 * If set this function is used to mutiplex between different test
> +	 * variants.
> +	 *
> +	 * Multiplexers are the way how to run the same test for different
> +	 * settings. The intended use is to test different syscall
> +	 * wrappers/variants but the API is generic and does not limit the
> +	 * usage in any way.
> +	 *
> +	 * Each time the function is called it switches to next test variant,
> +	 * returns zero if all variants has been iterated over.
> +	 */
> +	int (*test_multiplex)(void);
> +
>   	/* Minimal device size in megabytes */
>   	unsigned int dev_min_size;
>   
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 7dd890b8d..6b033c879 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1009,8 +1009,15 @@ static void testrun(void)
>   		if (!cont)
>   			break;
>   
> -		run_tests();
> -		heartbeat();
> +		if (tst_test->test_multiplex) {
> +			while (tst_test->test_multiplex()) {
> +				run_tests();
> +				heartbeat();
> +			}
> +		} else {
> +			run_tests();
> +			heartbeat();
> +		}
>   	}
>   
>   	do_test_cleanup();
> 

[LTP] [RFC PATCH 1/2] tst_test: Add test multiplex function

[Cyril Hrubis]

Hi!
> > > If this wasn't timer test, I'd ask why don't we use existing .test and
> > > .tcnt,
> > > your test() func can be called with a parameter, so you could change
> > > the code to choose correct syscall/glibc func based on value of that
> > > parameter.
> > > 
> > > For normal tests, this looks almost and .test/.tcnt functionality,
> > > except test count can be also dynamic.
> > 
> > Well so does the .all_filesystems flag, however I see these concepts to
> > be orthogonal to what the actual test does, which is the reason I want
> > them hooked up in the library rather than to be part of the testcase
> > code.
> 
> OK, so it's like another level on top of current test functions.

Yes, the change to the library adds generic layer for that.

> I probably would like 'variant' somewhere in name more, but overall
> I'm not against the patch. Are you planning on adding some docs too?

I thought that calling it multiplex was descriptive enough name.


There is some summary in:

https://github.com/linux-test-project/ltp/issues/506

I guess that I can expand the text a bit and put it in the doc/
directory in the source tree along with the change to the test library.

-- 
Cyril Hrubis
chrubis@suse.cz