Hi Shaopeng,
On 2/12/2023 10:24 PM, Shaopeng Tan wrote:
> After creating a child process with fork() in CAT test, if an error
> occurs when parent process runs cat_val() or check_results(), the child
> process will not be killed and also resctrlfs is not unmounted. Also if
> an error occurs when child process runs cat_val() or check_results(),
> the child process is returned, but the parent process will wait pipe
"child process is returned" -> "child process returns"
> message from child.
"will wait pipe message from child" -> "will wait for the pipe message
from the child"
> Synchronize the exits between the parent and child. An error occurs
> whether in parents process or child process, the parents process
> always kills child process and runs umount_resctrlfs(), and the
> child process always waits to be killed by the parent process.
I think the above could be easier to read with a few slight changes:
"
Synchronize the exits between the parent and child. An error could
occur whether in parent process or child process. The parent process
always kills the child process and runs umount_resctrlfs(). The
child process always waits to be killed by the parent process."
>
> Signed-off-by: Shaopeng Tan <tan.shaopeng@xxxxxxxxxxxxxx>
> ---
> tools/testing/selftests/resctrl/cat_test.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index 6a8306b0a109..477b62dac546 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -186,23 +186,20 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
> remove(param.filename);
>
> ret = cat_val(¶m);
> - if (ret)
> - return ret;
> -
> - ret = check_results(¶m);
> - if (ret)
> - return ret;
> + if (ret == 0)
> + ret = check_results(¶m);
>
> if (bm_pid == 0) {
> /* Tell parent that child is ready */
> close(pipefd[0]);
> pipe_message = 1;
> if (write(pipefd[1], &pipe_message, sizeof(pipe_message)) <
> - sizeof(pipe_message)) {
> - close(pipefd[1]);
> + sizeof(pipe_message))
> + /*
> + * Just print the error message.
> + * Let while(1) run and wait for itself to be killed.
> + */
> perror("# failed signaling parent process");
> - return errno;
> - }
>
> close(pipefd[1]);
> while (1)
> @@ -226,5 +223,5 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
> if (bm_pid)
> umount_resctrlfs();
>
> - return 0;
> + return ret;
> }
Apart from the changelog comments:
Reviewed-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
Thank you very much
Reinette
