[LTP] [PATCH] syscalls/gettid02: fix s390x and couple races

[Li Wang]

On Thu, Oct 19, 2023 at 7:40 PM Jan Stancek <jstancek@redhat.com> wrote:

> s390x is big endian where sizeof(int) == 4 and sizeof(void *) == 8.
> This test currently fails on s390x because in pthread_join, "&tids[i]"
> is treated as "void**" and due to different type size it writes over
> 2 entries of tids[] array. So for small tid values test reports:
>         gettid02.c:29: TPASS: Expect: parent ID (14048) differs from
> thread[0] ID (14049)
>         gettid02.c:29: TPASS: Expect: parent ID (14048) differs from
> thread[1] ID (14050)
>         gettid02.c:29: TPASS: Expect: parent ID (14048) differs from
> thread[2] ID (14051)
>         gettid02.c:29: TPASS: Expect: parent ID (14048) differs from
> thread[3] ID (14052)
>         gettid02.c:29: TPASS: Expect: parent ID (14048) differs from
> thread[4] ID (14053)
>         gettid02.c:29: TPASS: Expect: parent ID (14048) differs from
> thread[5] ID (14054)
>         gettid02.c:29: TPASS: Expect: parent ID (14048) differs from
> thread[6] ID (14055)
>         gettid02.c:29: TPASS: Expect: parent ID (14048) differs from
> thread[7] ID (14056)
>         gettid02.c:29: TPASS: Expect: parent ID (14048) differs from
> thread[8] ID (14057)
>         gettid02.c:29: TPASS: Expect: parent ID (14048) differs from
> thread[9] ID (14058)
>         gettid02.c:49: TINFO: thread[0] and thread[1] have the same ID
>         gettid02.c:49: TINFO: thread[0] and thread[2] have the same ID
>         gettid02.c:49: TINFO: thread[0] and thread[3] have the same ID
>         ...
> which is clearly wrong, since each thread above printed different ID.
>
> This construct is race-y on slower s390x systems:
>         for (int i = 0; i < THREADS_NUM; i++)
>                 SAFE_PTHREAD_CREATE(&thread, NULL, threaded, &i);
> because by the time thread starts and looks at "&i", the loop can
> already move on and increment "i".
>
> Also, potentially starting and waiting for each thread could be an issue,
> since kernel is
> free to re-use tid for already finished threads.
>

Make sense!

Reviewed-by: Li Wang <liwang@redhat.com>




>
> Instead of passing tid via pthread_join, just use already available global
> array "tids".
> Make sure the argument to pthread_create doesn't change, by creating an
> args array.
> Start all threads before we join any.
> And also print the value of TID in cases where test detects two threads
> use same one.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  testcases/kernel/syscalls/gettid/gettid02.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/gettid/gettid02.c
> b/testcases/kernel/syscalls/gettid/gettid02.c
> index 61f115fc9e7c..ef44761c41de 100644
> --- a/testcases/kernel/syscalls/gettid/gettid02.c
> +++ b/testcases/kernel/syscalls/gettid/gettid02.c
> @@ -16,7 +16,7 @@
>
>  #define THREADS_NUM 10
>
> -static pid_t tids[THREADS_NUM];
> +static volatile pid_t tids[THREADS_NUM];
>
>  static void *threaded(void *arg)
>  {
> @@ -29,24 +29,27 @@ static void *threaded(void *arg)
>         TST_EXP_EXPR(pid != tid,
>                 "parent ID (%d) differs from thread[%d] ID (%d)",
>                 pid, i, tid);
> -
> -       return (void *)tst_syscall(__NR_gettid);
> +       tids[i] = tid;
> +       return NULL;
>  }
>
>  static void run(void)
>  {
> -       pthread_t thread;
> +       pthread_t thread[THREADS_NUM];
> +       int args[THREADS_NUM];
>         int error = 0;
>
>         for (int i = 0; i < THREADS_NUM; i++) {
> -               SAFE_PTHREAD_CREATE(&thread, NULL, threaded, &i);
> -               SAFE_PTHREAD_JOIN(thread, (void **)&tids[i]);
> +               args[i] = i;
> +               SAFE_PTHREAD_CREATE(&thread[i], NULL, threaded, &args[i]);
>         }
> +       for (int i = 0; i < THREADS_NUM; i++)
> +               SAFE_PTHREAD_JOIN(thread[i], NULL);
>
>         for (int i = 0; i < THREADS_NUM; i++) {
>                 for (int j = i + 1; j < THREADS_NUM; j++) {
>                         if (tids[i] == tids[j]) {
> -                               tst_res(TINFO, "thread[%d] and thread[%d]
> have the same ID", i, j);
> +                               tst_res(TINFO, "thread[%d] and thread[%d]
> have the same ID %d", i, j, tids[i]);
>                                 error = 1;
>                         }
>                 }
> --
> 2.31.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang