[LTP] [PATCH 0/2] First step in formalizing the test library

[LTP] [PATCH 0/2] First step in formalizing the test library

[Cyril Hrubis]

This is partly motivated by the issue #462 where we ended up with a
loose corer cases that are not well defined.

So as a first step this patches:

- Do fix old problem with TBROK propagation

- Start a library description that hopefully captures
  so-far unwritten desing decisions and rules

Cyril Hrubis (2):
  lib: tst_test.c: Add TBROK counter.
  lib: Add test library design document

 lib/README.md               | 130 ++++++++++++++++++++++++++++++++++++
 lib/newlib_tests/.gitignore |   1 +
 lib/newlib_tests/test22.c   |  34 ++++++++++
 lib/tst_test.c              |  13 +++-
 4 files changed, 176 insertions(+), 2 deletions(-)
 create mode 100644 lib/README.md
 create mode 100644 lib/newlib_tests/test22.c

-- 
2.26.2

[LTP] [PATCH 1/2] lib: tst_test.c: Add TBROK counter.

[Cyril Hrubis]

This ensures that TBROK is never lost.

If test process forks a child and the child calls SAFE_MACRO() the
failure will be lost unless the test process handles the exit value
properly and propagates the TBROK.

It is also strange that TBROK is the only return value that is solely
propagated by the exit value and not by the counters. This has been
mistake to begin with.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 lib/newlib_tests/.gitignore |  1 +
 lib/newlib_tests/test22.c   | 34 ++++++++++++++++++++++++++++++++++
 lib/tst_test.c              | 13 +++++++++++--
 3 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 lib/newlib_tests/test22.c

diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore
index ac1d19be0..6fc549cf2 100644
--- a/lib/newlib_tests/.gitignore
+++ b/lib/newlib_tests/.gitignore
@@ -25,6 +25,7 @@ test18
 test19
 test20
 test21
+test22
 tst_expiration_timer
 test_assert
 test_timer
diff --git a/lib/newlib_tests/test22.c b/lib/newlib_tests/test22.c
new file mode 100644
index 000000000..520b8dad8
--- /dev/null
+++ b/lib/newlib_tests/test22.c
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*
+ * Test that TBROK is propagated correctly to the results even if we wait on
+ * child and throw away the status.
+ */
+#include "tst_test.h"
+
+static void do_test(void)
+{
+	int pid = SAFE_FORK();
+
+	if (pid) {
+		tst_res(TPASS, "Test main pid");
+		SAFE_WAITPID(pid, NULL, 0);
+		return;
+	}
+
+	if (tst_variant == 1)
+		tst_brk(TBROK, "Test child!");
+	else
+		tst_brk(TCONF, "Test child!");
+
+	tst_res(TPASS, "Test child");
+}
+
+static struct tst_test test = {
+	.test_all = do_test,
+	.test_variants = 2,
+	.forks_child = 1,
+};
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 535c0ff4c..128058e6e 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -56,6 +56,7 @@ struct results {
 	int skipped;
 	int failed;
 	int warnings;
+	int broken;
 	unsigned int timeout;
 };
 
@@ -179,6 +180,9 @@ static void update_results(int ttype)
 	case TFAIL:
 		tst_atomic_inc(&results->failed);
 	break;
+	case TBROK:
+		tst_atomic_inc(&results->broken);
+	break;
 	}
 }
 
@@ -368,10 +372,8 @@ static void check_child_status(pid_t pid, int status)
 	ret = WEXITSTATUS(status);
 	switch (ret) {
 	case TPASS:
-	break;
 	case TBROK:
 	case TCONF:
-		tst_brk(ret, "Reported by child (%i)", pid);
 	break;
 	default:
 		tst_brk(TBROK, "Invalid child (%i) exit value %i", pid, ret);
@@ -698,9 +700,13 @@ static void do_exit(int ret)
 		if (results->warnings)
 			ret |= TWARN;
 
+		if (results->broken)
+			ret |= TBROK;
+
 		printf("\nSummary:\n");
 		printf("passed   %d\n", results->passed);
 		printf("failed   %d\n", results->failed);
+		printf("broken   %d\n", results->broken);
 		printf("skipped  %d\n", results->skipped);
 		printf("warnings %d\n", results->warnings);
 	}
@@ -737,6 +743,9 @@ static int results_equal(struct results *a, struct results *b)
 	if (a->skipped != b->skipped)
 		return 0;
 
+	if (a->broken != b->broken)
+		return 0;
+
 	return 1;
 }
 
-- 
2.26.2

[LTP] [PATCH 1/2] lib: tst_test.c: Add TBROK counter.

[Cyril Hrubis]

Hi!
> This ensures that TBROK is never lost.
> 
> If test process forks a child and the child calls SAFE_MACRO() the
> failure will be lost unless the test process handles the exit value
> properly and propagates the TBROK.
> 
> It is also strange that TBROK is the only return value that is solely
> propagated by the exit value and not by the counters. This has been
> mistake to begin with.

Anybody to review this?

I consider it important and at the same time quite safe to apply since
I've tried to keep the changes minimal.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/2] lib: tst_test.c: Add TBROK counter.

[Petr Vorel]

> Hi!
> > This ensures that TBROK is never lost.

> > If test process forks a child and the child calls SAFE_MACRO() the
> > failure will be lost unless the test process handles the exit value
> > properly and propagates the TBROK.

> > It is also strange that TBROK is the only return value that is solely
> > propagated by the exit value and not by the counters. This has been
> > mistake to begin with.

> Anybody to review this?

> I consider it important and at the same time quite safe to apply since
> I've tried to keep the changes minimal.

+1

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

[LTP] [PATCH 1/2] lib: tst_test.c: Add TBROK counter.

[Cyril Hrubis]

Hi!
> Reviewed-by: Petr Vorel <pvorel@suse.cz>

Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] lib: Add test library design document

[Cyril Hrubis]

Which tries to explain high level overview and design choices for the
test library.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 lib/README.md | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 130 insertions(+)
 create mode 100644 lib/README.md

diff --git a/lib/README.md b/lib/README.md
new file mode 100644
index 000000000..6efd3cf33
--- /dev/null
+++ b/lib/README.md
@@ -0,0 +1,130 @@
+# Test library design document
+
+## Test lifetime overview
+
+When a test is executed the very first thing to happen is that the we check for
+various test pre-requisities. These are described in the tst\_test structure
+and range from simple '.require\_root' to a more complicated kernel .config
+boolean expressions such as:
+"CONFIG\_X86\_INTEL\_UMIP=y | CONFIG\_X86\_UMIP=y".
+
+If all checks are passed the process carries on with setting up the test
+environment as requested in the tst\_test structure. There are many different
+setup steps that have been put into the test library again ranging from rather
+simple creation of a unique test temporary directory to a bit more complicated
+ones such as preparing, formatting, and mounting a block device.
+
+The test library also intializes shrared memory used for IPC at this step.
+
+Once all the prerequisities are checked and test environment has been prepared
+we can move on executing the testcase itself. The actual test is executed in a
+forked process, however there are a few hops before we get there.
+
+First of all there are test variants, which means that the test is re-executed
+several times with a slightly different settings. This is usually used to test
+a family of similar syscalls, where we test each of these syscalls exactly the
+same, but without re-executing the test binary itself. Test varianst are
+implemented as a simple global variable counter that gets increased on each
+iteration. In a case of syscall tests we switch between which syscall to call
+based on the global counter.
+
+Then there is all\_filesystems flag which is mostly the same as test variants
+but executes the test for each filesystem supported by the system. Note that we
+can get cartesian product between test variants and all filesystems as well.
+
+In a pseoudo code it could be expressed as:
+
+```
+for test_variants:
+	for all_filesystems:
+		fork_testrun()
+```
+
+Before we fork() the test process the test library sets up a timeout alarm and
+also a heartbeat signal handlers and also sets up an alarm(2) accordingly to
+the test timeout. When a test timeouts the test library gets SIGALRM and the
+alarm handler mercilesly kills all forked children by sending SIGKILL to the
+whole process group. The heartbeat handler is used by the test process to reset
+this timer for example when the test functions runs in a loop.
+
+With that done we finally fork() the test process. The test process firstly
+resets signal handlers and sets its pid to be a process group leader so that we
+can slaughter all children if needed. The test library proceeds with suspending
+itself in waitpid() syscall and waits for the child to finish at this point.
+
+The test process goes ahead and call the test setup() function if present in
+the tst\_test structure. It's important that we execute all test callbacks
+after we have forked the process, that way we cannot crash the test library
+process. The setup can also cause the the test to exit prematurely by either
+direct or indirect (SAFE\_MACROS()) call to tst\_brk().  In this case the
+fork\_testrun() function exits, but the loops for test variants or filesystems
+carries on.
+
+All that is left to be done is to actually execute the tests, what happnes now
+depends on the -i and -I command line parameters that can request that the
+run() or run\_all() callbacks are executed N times or for a N seconds. Again
+the test can exit at any time by direct or indirect call to tst\_brk().
+
+Once the test is finished all that is left for the test process is the test
+cleanup(). So if a there is a cleanup() callback in the tst\_test strucuture
+it's executed. Callback runs in a special context where the tst\_brk(TBROK,
+...) calls are converted into tst\_res(TWARN, ...) calls. This is because we
+found out that carrying up with partially broken cleanup is usually better
+option than exitting it in the middle.
+
+The test cleanup() is also called by the tst\_brk() handler in order to cleanup
+before exitting the test process, hence it must be able to cope even with
+partiall test setup. Usually it suffices to make sure to clean up only
+resources that already have been set up and to do that in an inverse order that
+we did in setup().
+
+Once the test process exits or leaves the run() or run\_all() function the test
+library wakes up from the waitpid() call, and checks if the test process
+exitted normally.
+
+Once the testrun is finished the test library does a cleanup() as well to clean
+up resources set up in the test library setup(), reports test results and
+finally exits the process.
+
+### Test library and fork()-ing
+
+Things are a bit more complicated when fork()-ing is involved, however the
+tests results are stored in a page of a shared memory and incremented by atomic
+operations, hence the results are stored rigth after the test reporting
+fucntion returns from the test library and the access is, by definition,
+race-free as well.
+
+On the other hand the test library, apart from sending a SIGKILL to the whole
+process group on timeout, does not track granchildren.
+
+This especially means that:
+
+- The test exits once the main test process exits.
+
+- While the test results are, by the desing, propagated to the test library
+  we may still miss a child that gets killed by a signal or exits unexpectedly.
+
+The test writer should, because of these, take care for mourning these proceses
+properly, in most cases this could be simply done by calling
+tst\_reap\_children() to collect and dissect deceased.
+
+Also note that tst\_brk() does exit only the current process, so if child
+process calls tst\_brk() the counters are incremented and the process exits.
+
+### Test library and exec()
+
+The piece of mapped memory to store the results to is not preserved over
+exec(2), hence to use the test library from a binary started by an exec() it
+has to be remaped. In this case the process must to call tst\_reinit() before
+calling any other library functions. In order to make this happen the program
+environment carries LTP\_IPC\_PATH variable with a path to the backing file on
+tmpfs. This also allows us to use the test library from a shell testcases.
+
+### Test library and process synchronization
+
+The piece of mapped memory is also used as a base for a futex-based
+synchronization primitives called checkpoints. And as said previously the
+memory can be mapped to any process by calling the tst\_reinit() function. As a
+matter of a fact there is even a tst\_checkpoint binary that allows use to use
+the checkpoints from shell code as well.
+
-- 
2.26.2

[LTP] [PATCH 2/2] lib: Add test library design document

[Jan Stancek]

On Fri, Nov 27, 2020 at 05:31:50PM +0100, Cyril Hrubis wrote:
>Which tries to explain high level overview and design choices for the
>test library.
>
>Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
>---
> lib/README.md | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 130 insertions(+)
> create mode 100644 lib/README.md
>
>diff --git a/lib/README.md b/lib/README.md
>new file mode 100644
>index 000000000..6efd3cf33
>--- /dev/null
>+++ b/lib/README.md
>@@ -0,0 +1,130 @@
>+# Test library design document
>+
>+## Test lifetime overview
>+
>+When a test is executed the very first thing to happen is that the we check for
>+various test pre-requisities. These are described in the tst\_test structure
>+and range from simple '.require\_root' to a more complicated kernel .config
>+boolean expressions such as:
>+"CONFIG\_X86\_INTEL\_UMIP=y | CONFIG\_X86\_UMIP=y".
>+
>+If all checks are passed the process carries on with setting up the test
>+environment as requested in the tst\_test structure. There are many different
>+setup steps that have been put into the test library again ranging from rather
>+simple creation of a unique test temporary directory to a bit more complicated
>+ones such as preparing, formatting, and mounting a block device.
>+
>+The test library also intializes shrared memory used for IPC at this step.
>+
>+Once all the prerequisities are checked and test environment has been prepared
>+we can move on executing the testcase itself. The actual test is executed in a
>+forked process, however there are a few hops before we get there.
>+
>+First of all there are test variants, which means that the test is re-executed
>+several times with a slightly different settings. This is usually used to test
>+a family of similar syscalls, where we test each of these syscalls exactly the
>+same, but without re-executing the test binary itself. Test varianst are
>+implemented as a simple global variable counter that gets increased on each
>+iteration. In a case of syscall tests we switch between which syscall to call
>+based on the global counter.
>+
>+Then there is all\_filesystems flag which is mostly the same as test variants
>+but executes the test for each filesystem supported by the system. Note that we
>+can get cartesian product between test variants and all filesystems as well.
>+
>+In a pseoudo code it could be expressed as:
>+
>+```
>+for test_variants:
>+	for all_filesystems:
>+		fork_testrun()
>+```
>+
>+Before we fork() the test process the test library sets up a timeout alarm and
>+also a heartbeat signal handlers and also sets up an alarm(2) accordingly to
>+the test timeout. When a test timeouts the test library gets SIGALRM and the
>+alarm handler mercilesly kills all forked children by sending SIGKILL to the
>+whole process group. The heartbeat handler is used by the test process to reset
>+this timer for example when the test functions runs in a loop.
>+
>+With that done we finally fork() the test process. The test process firstly
>+resets signal handlers and sets its pid to be a process group leader so that we
>+can slaughter all children if needed. The test library proceeds with suspending
>+itself in waitpid() syscall and waits for the child to finish at this point.
>+
>+The test process goes ahead and call the test setup() function if present in
>+the tst\_test structure. It's important that we execute all test callbacks
>+after we have forked the process, that way we cannot crash the test library
>+process. The setup can also cause the the test to exit prematurely by either
>+direct or indirect (SAFE\_MACROS()) call to tst\_brk().  In this case the
>+fork\_testrun() function exits, but the loops for test variants or filesystems
>+carries on.
>+
>+All that is left to be done is to actually execute the tests, what happnes now
>+depends on the -i and -I command line parameters that can request that the
>+run() or run\_all() callbacks are executed N times or for a N seconds. Again
>+the test can exit at any time by direct or indirect call to tst\_brk().
>+
>+Once the test is finished all that is left for the test process is the test
>+cleanup(). So if a there is a cleanup() callback in the tst\_test strucuture
>+it's executed. Callback runs in a special context where the tst\_brk(TBROK,
>+...) calls are converted into tst\_res(TWARN, ...) calls. This is because we
>+found out that carrying up with partially broken cleanup is usually better
>+option than exitting it in the middle.
>+
>+The test cleanup() is also called by the tst\_brk() handler in order to cleanup
>+before exitting the test process, hence it must be able to cope even with
>+partiall test setup. Usually it suffices to make sure to clean up only
>+resources that already have been set up and to do that in an inverse order that
>+we did in setup().
>+
>+Once the test process exits or leaves the run() or run\_all() function the test
>+library wakes up from the waitpid() call, and checks if the test process
>+exitted normally.
>+
>+Once the testrun is finished the test library does a cleanup() as well to clean
>+up resources set up in the test library setup(), reports test results and
>+finally exits the process.
>+
>+### Test library and fork()-ing
>+
>+Things are a bit more complicated when fork()-ing is involved, however the
>+tests results are stored in a page of a shared memory and incremented by atomic
>+operations, hence the results are stored rigth after the test reporting
>+fucntion returns from the test library and the access is, by definition,
>+race-free as well.
>+
>+On the other hand the test library, apart from sending a SIGKILL to the whole
>+process group on timeout, does not track granchildren.
>+
>+This especially means that:
>+
>+- The test exits once the main test process exits.
>+
>+- While the test results are, by the desing, propagated to the test library
                                       ^^ typo

>+  we may still miss a child that gets killed by a signal or exits unexpectedly.
>+
>+The test writer should, because of these, take care for mourning these proceses
>+properly, in most cases this could be simply done by calling
>+tst\_reap\_children() to collect and dissect deceased.
>+
>+Also note that tst\_brk() does exit only the current process, so if child
>+process calls tst\_brk() the counters are incremented and the process exits.
>+
>+### Test library and exec()
>+
>+The piece of mapped memory to store the results to is not preserved over
>+exec(2), hence to use the test library from a binary started by an exec() it
>+has to be remaped. In this case the process must to call tst\_reinit() before
>+calling any other library functions. In order to make this happen the program
>+environment carries LTP\_IPC\_PATH variable with a path to the backing file on
>+tmpfs. This also allows us to use the test library from a shell testcases.
>+
>+### Test library and process synchronization
>+
>+The piece of mapped memory is also used as a base for a futex-based
>+synchronization primitives called checkpoints. And as said previously the
>+memory can be mapped to any process by calling the tst\_reinit() function. As a
>+matter of a fact there is even a tst\_checkpoint binary that allows use to use
>+the checkpoints from shell code as well.
>+

Looks good to me.

What do you think about adding a small ascii picture(s)?
For example, one that shows outline of what's called in
library vs. test process:

        lib process                                                                                    
        +----------------------------+                                                                 
        | main                       |                                                                 
        |  tst_run_tcases            |                                                                 
        |   do_setup                 |                                                                 
        |   for_each_variant         |                                                                 
        |    for_each_filesystem     |          test process                                           
        |     fork_testrun ---------------------+--------------------------------------------+         
        |      waitpid               |          | testrun                                    |         
        |                            |          |  do_test_setup                             |         
        |                            |          |   tst_test->setup                          |         
        |                            |          |  run_tests                                 |         
        |                            |          |   tst_test->test(i) or tst_test->test_all  |         
        |                            |          |  do_test_cleanup                           |         
        |                            |          |   tst_test->cleanup                        |         
        |                            |          |  exit(0)                                   |         
        |   do_exit                  |          +--------------------------------------------+         
        |    do_cleanup              |                                                                 
        |     exit(ret)              |                                                                 
        +----------------------------+                                                                 

[LTP] [PATCH 2/2] lib: Add test library design document

[Cyril Hrubis]

Hi!
> Looks good to me.

Thanks.

> What do you think about adding a small ascii picture(s)?
> For example, one that shows outline of what's called in
> library vs. test process:
> 
>         lib process                                                                                    
>         +----------------------------+                                                                 
>         | main                       |                                                                 
>         |  tst_run_tcases            |                                                                 
>         |   do_setup                 |                                                                 
>         |   for_each_variant         |                                                                 
>         |    for_each_filesystem     |          test process                                           
>         |     fork_testrun ---------------------+--------------------------------------------+         
>         |      waitpid               |          | testrun                                    |         
>         |                            |          |  do_test_setup                             |         
>         |                            |          |   tst_test->setup                          |         
>         |                            |          |  run_tests                                 |         
>         |                            |          |   tst_test->test(i) or tst_test->test_all  |         
>         |                            |          |  do_test_cleanup                           |         
>         |                            |          |   tst_test->cleanup                        |         
>         |                            |          |  exit(0)                                   |         
>         |   do_exit                  |          +--------------------------------------------+         
>         |    do_cleanup              |                                                                 
>         |     exit(ret)              |                                                                 
>         +----------------------------+                                                                 

I would love that, feel free to send v2 based on my patch.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 2/2] lib: Add test library design document

[Jan Stancek]

From: Cyril Hrubis <chrubis@suse.cz>

Which tries to explain high level overview and design choices for the
test library (also know as "newlib").

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
Acked-by: Jan Stancek <jstancek@redhat.com>
---
 lib/README.md | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)
 create mode 100644 lib/README.md

Add ascii picture and fix small typo.

diff --git a/lib/README.md b/lib/README.md
new file mode 100644
index 000000000000..2b81ec9aea33
--- /dev/null
+++ b/lib/README.md
@@ -0,0 +1,153 @@
+# Test library design document
+
+## High-level picture
+
+    library process
+    +----------------------------+
+    | main                       |
+    |  tst_run_tcases            |
+    |   do_setup                 |
+    |   for_each_variant         |
+    |    for_each_filesystem     |   test process
+    |     fork_testrun ------------->+--------------------------------------------+
+    |      waitpid               |   | testrun                                    |
+    |                            |   |  do_test_setup                             |
+    |                            |   |   tst_test->setup                          |
+    |                            |   |  run_tests                                 |
+    |                            |   |   tst_test->test(i) or tst_test->test_all  |
+    |                            |   |  do_test_cleanup                           |
+    |                            |   |   tst_test->cleanup                        |
+    |                            |   |  exit(0)                                   |
+    |   do_exit                  |   +--------------------------------------------+
+    |    do_cleanup              |
+    |     exit(ret)              |
+    +----------------------------+
+
+## Test lifetime overview
+
+When a test is executed the very first thing to happen is that the we check for
+various test pre-requisities. These are described in the tst\_test structure
+and range from simple '.require\_root' to a more complicated kernel .config
+boolean expressions such as:
+"CONFIG\_X86\_INTEL\_UMIP=y | CONFIG\_X86\_UMIP=y".
+
+If all checks are passed the process carries on with setting up the test
+environment as requested in the tst\_test structure. There are many different
+setup steps that have been put into the test library again ranging from rather
+simple creation of a unique test temporary directory to a bit more complicated
+ones such as preparing, formatting, and mounting a block device.
+
+The test library also intializes shrared memory used for IPC at this step.
+
+Once all the prerequisities are checked and test environment has been prepared
+we can move on executing the testcase itself. The actual test is executed in a
+forked process, however there are a few hops before we get there.
+
+First of all there are test variants, which means that the test is re-executed
+several times with a slightly different settings. This is usually used to test
+a family of similar syscalls, where we test each of these syscalls exactly the
+same, but without re-executing the test binary itself. Test varianst are
+implemented as a simple global variable counter that gets increased on each
+iteration. In a case of syscall tests we switch between which syscall to call
+based on the global counter.
+
+Then there is all\_filesystems flag which is mostly the same as test variants
+but executes the test for each filesystem supported by the system. Note that we
+can get cartesian product between test variants and all filesystems as well.
+
+In a pseoudo code it could be expressed as:
+
+```
+for test_variants:
+	for all_filesystems:
+		fork_testrun()
+```
+
+Before we fork() the test process the test library sets up a timeout alarm and
+also a heartbeat signal handlers and also sets up an alarm(2) accordingly to
+the test timeout. When a test timeouts the test library gets SIGALRM and the
+alarm handler mercilesly kills all forked children by sending SIGKILL to the
+whole process group. The heartbeat handler is used by the test process to reset
+this timer for example when the test functions runs in a loop.
+
+With that done we finally fork() the test process. The test process firstly
+resets signal handlers and sets its pid to be a process group leader so that we
+can slaughter all children if needed. The test library proceeds with suspending
+itself in waitpid() syscall and waits for the child to finish at this point.
+
+The test process goes ahead and call the test setup() function if present in
+the tst\_test structure. It's important that we execute all test callbacks
+after we have forked the process, that way we cannot crash the test library
+process. The setup can also cause the the test to exit prematurely by either
+direct or indirect (SAFE\_MACROS()) call to tst\_brk().  In this case the
+fork\_testrun() function exits, but the loops for test variants or filesystems
+carries on.
+
+All that is left to be done is to actually execute the tests, what happnes now
+depends on the -i and -I command line parameters that can request that the
+run() or run\_all() callbacks are executed N times or for a N seconds. Again
+the test can exit at any time by direct or indirect call to tst\_brk().
+
+Once the test is finished all that is left for the test process is the test
+cleanup(). So if a there is a cleanup() callback in the tst\_test strucuture
+it's executed. Callback runs in a special context where the tst\_brk(TBROK,
+...) calls are converted into tst\_res(TWARN, ...) calls. This is because we
+found out that carrying up with partially broken cleanup is usually better
+option than exitting it in the middle.
+
+The test cleanup() is also called by the tst\_brk() handler in order to cleanup
+before exitting the test process, hence it must be able to cope even with
+partiall test setup. Usually it suffices to make sure to clean up only
+resources that already have been set up and to do that in an inverse order that
+we did in setup().
+
+Once the test process exits or leaves the run() or run\_all() function the test
+library wakes up from the waitpid() call, and checks if the test process
+exitted normally.
+
+Once the testrun is finished the test library does a cleanup() as well to clean
+up resources set up in the test library setup(), reports test results and
+finally exits the process.
+
+### Test library and fork()-ing
+
+Things are a bit more complicated when fork()-ing is involved, however the
+tests results are stored in a page of a shared memory and incremented by atomic
+operations, hence the results are stored rigth after the test reporting
+fucntion returns from the test library and the access is, by definition,
+race-free as well.
+
+On the other hand the test library, apart from sending a SIGKILL to the whole
+process group on timeout, does not track granchildren.
+
+This especially means that:
+
+- The test exits once the main test process exits.
+
+- While the test results are, by the design, propagated to the test library
+  we may still miss a child that gets killed by a signal or exits unexpectedly.
+
+The test writer should, because of these, take care for mourning these proceses
+properly, in most cases this could be simply done by calling
+tst\_reap\_children() to collect and dissect deceased.
+
+Also note that tst\_brk() does exit only the current process, so if child
+process calls tst\_brk() the counters are incremented and the process exits.
+
+### Test library and exec()
+
+The piece of mapped memory to store the results to is not preserved over
+exec(2), hence to use the test library from a binary started by an exec() it
+has to be remaped. In this case the process must to call tst\_reinit() before
+calling any other library functions. In order to make this happen the program
+environment carries LTP\_IPC\_PATH variable with a path to the backing file on
+tmpfs. This also allows us to use the test library from a shell testcases.
+
+### Test library and process synchronization
+
+The piece of mapped memory is also used as a base for a futex-based
+synchronization primitives called checkpoints. And as said previously the
+memory can be mapped to any process by calling the tst\_reinit() function. As a
+matter of a fact there is even a tst\_checkpoint binary that allows use to use
+the checkpoints from shell code as well.
+
-- 
2.18.1

[LTP] [PATCH v2 2/2] lib: Add test library design document

[Cyril Hrubis]

Hi!
> +## High-level picture
> +
> +    library process
> +    +----------------------------+
> +    | main                       |
> +    |  tst_run_tcases            |
> +    |   do_setup                 |
> +    |   for_each_variant         |
> +    |    for_each_filesystem     |   test process
> +    |     fork_testrun ------------->+--------------------------------------------+
> +    |      waitpid               |   | testrun                                    |
> +    |                            |   |  do_test_setup                             |
> +    |                            |   |   tst_test->setup                          |
> +    |                            |   |  run_tests                                 |
> +    |                            |   |   tst_test->test(i) or tst_test->test_all  |
> +    |                            |   |  do_test_cleanup                           |
> +    |                            |   |   tst_test->cleanup                        |
> +    |                            |   |  exit(0)                                   |
> +    |   do_exit                  |   +--------------------------------------------+
> +    |    do_cleanup              |
> +    |     exit(ret)              |
> +    +----------------------------+

Shouldn't we wrap this to ``` so that it renders nicely on the web?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 2/2] lib: Add test library design document

[Jan Stancek]

----- Original Message -----
> Hi!
> > +## High-level picture
> > +
> > +    library process
> > +    +----------------------------+
> > +    | main                       |
> > +    |  tst_run_tcases            |
> > +    |   do_setup                 |
> > +    |   for_each_variant         |
> > +    |    for_each_filesystem     |   test process
> > +    |     fork_testrun
> > ------------->+--------------------------------------------+
> > +    |      waitpid               |   | testrun
> > |
> > +    |                            |   |  do_test_setup
> > |
> > +    |                            |   |   tst_test->setup
> > |
> > +    |                            |   |  run_tests
> > |
> > +    |                            |   |   tst_test->test(i) or
> > tst_test->test_all  |
> > +    |                            |   |  do_test_cleanup
> > |
> > +    |                            |   |   tst_test->cleanup
> > |
> > +    |                            |   |  exit(0)
> > |
> > +    |   do_exit                  |
> > +--------------------------------------------+
> > +    |    do_cleanup              |
> > +    |     exit(ret)              |
> > +    +----------------------------+
> 
> Shouldn't we wrap this to ``` so that it renders nicely on the web?

I thought the 4 spaces prefix will already do that. I was testing
it online on dillinger.io and stackedit.io. But I'm fine with adding ```,
if that's better for github.

[LTP] [PATCH v2 2/2] lib: Add test library design document

[Cyril Hrubis]

Hi!
> > > +## High-level picture
> > > +
> > > +    library process
> > > +    +----------------------------+
> > > +    | main                       |
> > > +    |  tst_run_tcases            |
> > > +    |   do_setup                 |
> > > +    |   for_each_variant         |
> > > +    |    for_each_filesystem     |   test process
> > > +    |     fork_testrun
> > > ------------->+--------------------------------------------+
> > > +    |      waitpid               |   | testrun
> > > |
> > > +    |                            |   |  do_test_setup
> > > |
> > > +    |                            |   |   tst_test->setup
> > > |
> > > +    |                            |   |  run_tests
> > > |
> > > +    |                            |   |   tst_test->test(i) or
> > > tst_test->test_all  |
> > > +    |                            |   |  do_test_cleanup
> > > |
> > > +    |                            |   |   tst_test->cleanup
> > > |
> > > +    |                            |   |  exit(0)
> > > |
> > > +    |   do_exit                  |
> > > +--------------------------------------------+
> > > +    |    do_cleanup              |
> > > +    |     exit(ret)              |
> > > +    +----------------------------+
> > 
> > Shouldn't we wrap this to ``` so that it renders nicely on the web?
> 
> I thought the 4 spaces prefix will already do that. I was testing
> it online on dillinger.io and stackedit.io. But I'm fine with adding ```,
> if that's better for github.

My bad, four spaces should work here.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 2/2] lib: Add test library design document

[Joerg Vehlow]

Hi,

thanks for adding documentation, that is really useful.

some typos and a few comments

On 12/1/2020 10:07 AM, Jan Stancek wrote:
> From: Cyril Hrubis <chrubis@suse.cz>
>
> Which tries to explain high level overview and design choices for the
> test library (also know as "newlib").
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> Acked-by: Jan Stancek <jstancek@redhat.com>
> ---
>   lib/README.md | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 153 insertions(+)
>   create mode 100644 lib/README.md
>
> Add ascii picture and fix small typo.
>
> diff --git a/lib/README.md b/lib/README.md
> new file mode 100644
> index 000000000000..2b81ec9aea33
> --- /dev/null
> +++ b/lib/README.md
> @@ -0,0 +1,153 @@
> +# Test library design document
> +
> +## High-level picture
> +
> +    library process
> +    +----------------------------+
> +    | main                       |
> +    |  tst_run_tcases            |
> +    |   do_setup                 |
> +    |   for_each_variant         |
> +    |    for_each_filesystem     |   test process
> +    |     fork_testrun ------------->+--------------------------------------------+
> +    |      waitpid               |   | testrun                                    |
> +    |                            |   |  do_test_setup                             |
> +    |                            |   |   tst_test->setup                          |
> +    |                            |   |  run_tests                                 |
> +    |                            |   |   tst_test->test(i) or tst_test->test_all  |
> +    |                            |   |  do_test_cleanup                           |
> +    |                            |   |   tst_test->cleanup                        |
> +    |                            |   |  exit(0)                                   |
> +    |   do_exit                  |   +--------------------------------------------+
> +    |    do_cleanup              |
> +    |     exit(ret)              |
> +    +----------------------------+
> +
> +## Test lifetime overview
> +
> +When a test is executed the very first thing to happen is that the we check for
the we
> +various test pre-requisities. These are described in the tst\_test structure
prerequisites (better without hyphen and typo)
> +and range from simple '.require\_root' to a more complicated kernel .config
> +boolean expressions such as:
> +"CONFIG\_X86\_INTEL\_UMIP=y | CONFIG\_X86\_UMIP=y".
> +
> +If all checks are passed the process carries on with setting up the test
> +environment as requested in the tst\_test structure. There are many different
> +setup steps that have been put into the test library again ranging from rather
> +simple creation of a unique test temporary directory to a bit more complicated
> +ones such as preparing, formatting, and mounting a block device.
> +
> +The test library also intializes shrared memory used for IPC at this step.
> +
> +Once all the prerequisities are checked and test environment has been prepared
prerequisites
> +we can move on executing the testcase itself. The actual test is executed in a
> +forked process, however there are a few hops before we get there.
> +
> +First of all there are test variants, which means that the test is re-executed
> +several times with a slightly different settings. This is usually used to test
a setting
> +a family of similar syscalls, where we test each of these syscalls exactly the
> +same, but without re-executing the test binary itself. Test varianst are
variants
> +implemented as a simple global variable counter that gets increased on each
> +iteration. In a case of syscall tests we switch between which syscall to call
> +based on the global counter.
> +
> +Then there is all\_filesystems flag which is mostly the same as test variants
> +but executes the test for each filesystem supported by the system. Note that we
> +can get cartesian product between test variants and all filesystems as well.
> +
> +In a pseoudo code it could be expressed as:
> +
> +```
> +for test_variants:
> +	for all_filesystems:
> +		fork_testrun()
> +```
> +
> +Before we fork() the test process the test library sets up a timeout alarm and
> +also a heartbeat signal handlers and also sets up an alarm(2) accordingly to
> +the test timeout. When a test timeouts the test library gets SIGALRM and the
"times out", I guess there is no verb timeout
> +alarm handler mercilesly kills all forked children by sending SIGKILL to the
mercilessly
> +whole process group. The heartbeat handler is used by the test process to reset
> +this timer for example when the test functions runs in a loop.
either "function runs" or "functions run"
> +
> +With that done we finally fork() the test process. The test process firstly
> +resets signal handlers and sets its pid to be a process group leader so that we
> +can slaughter all children if needed. The test library proceeds with suspending
> +itself in waitpid() syscall and waits for the child to finish at this point.
> +
> +The test process goes ahead and call the test setup() function if present in
calls
> +the tst\_test structure. It's important that we execute all test callbacks
> +after we have forked the process, that way we cannot crash the test library
> +process. The setup can also cause the the test to exit prematurely by either
double the
> +direct or indirect (SAFE\_MACROS()) call to tst\_brk().  In this case the
> +fork\_testrun() function exits, but the loops for test variants or filesystems
> +carries on.
> +
> +All that is left to be done is to actually execute the tests, what happnes now
> +depends on the -i and -I command line parameters that can request that the
> +run() or run\_all() callbacks are executed N times or for a N seconds. Again
"for N seconds"
> +the test can exit at any time by direct or indirect call to tst\_brk().
> +
> +Once the test is finished all that is left for the test process is the test
> +cleanup(). So if a there is a cleanup() callback in the tst\_test strucuture
structure
> +it's executed. Callback runs in a special context where the tst\_brk(TBROK,
Which callbacks? Only the cleanup callback, right? Then this should be 
"This callback runs"
> +...) calls are converted into tst\_res(TWARN, ...) calls. This is because we
> +found out that carrying up with partially broken cleanup is usually better
"carrying on"
> +option than exitting it in the middle.
> +
> +The test cleanup() is also called by the tst\_brk() handler in order to cleanup
> +before exitting the test process, hence it must be able to cope even with
> +partiall test setup. Usually it suffices to make sure to clean up only
partial
> +resources that already have been set up and to do that in an inverse order that
> +we did in setup().
> +
> +Once the test process exits or leaves the run() or run\_all() function the test
> +library wakes up from the waitpid() call, and checks if the test process
> +exitted normally.
> +
> +Once the testrun is finished the test library does a cleanup() as well to clean
> +up resources set up in the test library setup(), reports test results and
> +finally exits the process.
> +
> +### Test library and fork()-ing
> +
> +Things are a bit more complicated when fork()-ing is involved, however the
> +tests results are stored in a page of a shared memory and incremented by atomic
either "test's results" or "test results"
> +operations, hence the results are stored rigth after the test reporting
right
> +fucntion returns from the test library and the access is, by definition,
function
> +race-free as well.
> +
> +On the other hand the test library, apart from sending a SIGKILL to the whole
> +process group on timeout, does not track granchildren.
grandchildren
> +
> +This especially means that:
> +
> +- The test exits once the main test process exits.
> +
> +- While the test results are, by the design, propagated to the test library
> +  we may still miss a child that gets killed by a signal or exits unexpectedly.
> +
> +The test writer should, because of these, take care for mourning these proceses
"because of this" and processes

mourning sounds strange maybe just "take care of these processes"?
> +properly, in most cases this could be simply done by calling
> +tst\_reap\_children() to collect and dissect deceased.
> +
> +Also note that tst\_brk() does exit only the current process, so if child
"if a child"
> +process calls tst\_brk() the counters are incremented and the process exits.
> +
> +### Test library and exec()
> +
> +The piece of mapped memory to store the results to is not preserved over
> +exec(2), hence to use the test library from a binary started by an exec() it
> +has to be remaped. In this case the process must to call tst\_reinit() before
> +calling any other library functions. In order to make this happen the program
> +environment carries LTP\_IPC\_PATH variable with a path to the backing file on
> +tmpfs. This also allows us to use the test library from a shell testcases.
"from shell testcases"
> +
> +### Test library and process synchronization
> +
> +The piece of mapped memory is also used as a base for a futex-based
> +synchronization primitives called checkpoints. And as said previously the
> +memory can be mapped to any process by calling the tst\_reinit() function. As a
> +matter of a fact there is even a tst\_checkpoint binary that allows use to use
"us to use"
> +the checkpoints from shell code as well.
> +

J?rg

[LTP] [PATCH v2 2/2] lib: Add test library design document

[Cyril Hrubis]

Hi!
> thanks for adding documentation, that is really useful.
> 
> some typos and a few comments

Fixed the typos and clarified bits you pointed out (and couple more I've
found today) and pushed, thanks a lot.

-- 
Cyril Hrubis
chrubis@suse.cz