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

[Jan Stancek]

On Fri, Oct 20, 2023 at 11:23 AM Li Wang <liwang@redhat.com> wrote:
>
>
>
> 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".
>
>
> Sorry, just come up one more question, pthread_join will stay there
> until the thread revokes finish, so why the loop can move on and increase i?

You're right, that comment applies only after create and join runs separately.
It was meant as explanation for the need for "args" array - I can
reword commit log.

>
>
>>
>>
>> Also, potentially starting and waiting for each thread could be an issue, since kernel is
>> free to re-use tid for already finished threads.
>>
>> 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