[LTP] [PATCH v2] lib: Improve doc

[Petr Vorel]

Hi Avinesh,

> Signed-off-by: Avinesh Kumar <akumar@suse.de>
> ---
>  lib/README.md | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)

> diff --git a/lib/README.md b/lib/README.md
> index ccb1cf1da..dd79e70f4 100644
> --- a/lib/README.md
> +++ b/lib/README.md
> @@ -27,16 +27,16 @@

>  When a test is executed the very first thing to happen is that we check for
>  various test prerequisites. These are described in the tst\_test structure and
> -range from simple '.require\_root' to a more complicated kernel .config boolean
> +range from simple '.needs\_root' to a more complicated kernel .config boolean
Good catch!

>  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
> +If all checks are passed, the process continues 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.
> +The test library also initializes shared memory used for IPC at this step.

>  Once all the prerequisites are checked and test environment has been prepared
>  we can move on executing the testcase itself. The actual test is executed in a
> @@ -62,12 +62,11 @@ for test_variants:
>  		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 times out the test library gets SIGALRM and the
> -alarm handler mercilessly 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 run in a loop.
> +Before we fork() the test process, the test library sets up a timeout alarm and
nit: "Before we fork " or "Before 'fork() '.

> +also a heartbeat signal handler. When a test times out, the test library gets

I'm not sure about this, isn't the original description correct? (1) timeout
alarm() 2) a heartbeat signal handlers 3) second alarm() accordingly to the test
timeout?

I mean there are 2 alarm() calls in tst_test.c.

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

Kind regards,
Petr

> +SIGALRM and the alarm handler mercilessly 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 run 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
> @@ -97,14 +96,14 @@ usually better option than exiting it in the middle.
>  The test cleanup() is also called by the tst\_brk() handler in order to cleanup
>  before exiting the test process, hence it must be able to cope even with
>  partial 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().
> +resources that already have been set up and to do that in the reverse 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
>  exited normally.

> -Once the testrun is finished the test library does a cleanup() as well to clean
> +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.

> @@ -126,8 +125,8 @@ This especially means that:
>  - 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 this, take care for reaping these proceses
> -properly, in most cases this could be simply done by calling
> +The test writer should, because of this, take care of reaping 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 a child
> @@ -136,9 +135,9 @@ exits.

>  ### Test library and exec()

> -The piece of mapped memory to store the results to is not preserved over
> +The piece of mapped memory to store the results 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
> +has to be remapped. In this case, the process must 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 shell testcases.
> @@ -148,5 +147,5 @@ tmpfs. This also allows us to use the test library from shell testcases.
>  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 us to use
> +matter of a fact, there is even a tst\_checkpoint binary that allows us to use
>  the checkpoints from shell code as well.