unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org>
To: Florian Weimer <fweimer@redhat.com>
Cc: libc-alpha@sourceware.org
Subject: Re: [PATCH v2 03/19] nptl: Handle robust PI mutexes for !__ASSUME_SET_ROBUST_LIST
Date: Thu, 26 Aug 2021 09:14:19 -0300	[thread overview]
Message-ID: <6f7d9204-6a81-a021-30b4-e3bbea580fae@linaro.org> (raw)
In-Reply-To: <87lf4o8rux.fsf@oldenburg.str.redhat.com>



On 26/08/2021 06:42, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> The robust PI mutexes are signaled by setting the LSB bit to 1, so
>> the code requires to take this consideration before access the
>> __pthread_mutex_s.
>>
>> The code is also simplified: the initialization code is not really
>> required, PD->robust_head.list and PD->robust_list.__next are
>> essentially the same regardless of __PTHREAD_MUTEX_HAVE_PREV, the futex
>> wake is optimized to be issued only when required, and the futex shared
>> bit is set only when required.
> 
> Is this a user-visible bug?  Should it have a bug reference?

This a bug on only visible on a handful of architectures where 
__NR_set_robust_list is not support due the missing CONFIG_HAVE_FUTEX_CMPXCHG.
I have opened https://sourceware.org/bugzilla/show_bug.cgi?id=28268

> 
>> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
>> index d8ec299cb1..08e5189ad6 100644
>> --- a/nptl/pthread_create.c
>> +++ b/nptl/pthread_create.c
>> @@ -486,35 +486,36 @@ start_thread (void *arg)
>>      exit (0);
>>  
>>  #ifndef __ASSUME_SET_ROBUST_LIST
>> -  /* If this thread has any robust mutexes locked, handle them now.  */
>> -# if __PTHREAD_MUTEX_HAVE_PREV
>> -  void *robust = pd->robust_head.list;
>> -# else
>> -  __pthread_slist_t *robust = pd->robust_list.__next;
>> -# endif
>> -  /* We let the kernel do the notification if it is able to do so.
>> -     If we have to do it here there for sure are no PI mutexes involved
>> -     since the kernel support for them is even more recent.  */
>> -  if (!__nptl_set_robust_list_avail
>> -      && __builtin_expect (robust != (void *) &pd->robust_head, 0))
>> +  /* We let the kernel do the notification if it is able to do so on the exit
>> +     syscall.  Otherwise we need to handle before the thread terminates.  */
>> +  void **robust;
>> +  while ((robust = pd->robust_head.list)
>> +	 && robust != (void *) &pd->robust_head)
>>      {
>> -      do
>> +      /* Note: robust PI futexes are signaled by setting bit 0.  */
>> +      void **robustp = (void **) ((uintptr_t) robust & ~1UL);
>> +
>> +      struct __pthread_mutex_s *mtx = (struct __pthread_mutex_s *)
>> +	((char *) robustp - offsetof (struct __pthread_mutex_s,
>> +				      __list.__next));
>> +      unsigned int nusers = mtx->__nusers;
>> +      int shared = mtx->__kind & 128;
>> +
>> +      pd->robust_head.list_op_pending = robust;
>> +      pd->robust_head.list = *robustp;
>> +      /* Although the list will not be changed at this point, it follows the
>> +         expected kernel ABI.  */
>> +      __asm ("" ::: "memory");
>> +
>> +      int lock = atomic_exchange_relaxed (&mtx->__lock, FUTEX_OWNER_DIED);
>> +      /* Wake any users if mutex is acquired with potential users.  */
>> +      if (lock > 1 || nusers != 0)
> 
> Why the check for nusers?  Isn't that racy?

I was small optimization to avoid calling the futex, but I think you are
correct that is racy.  I think it would be better to let the kernel handle
it.

> 
> Thanks,
> Florian
> 

  reply	other threads:[~2021-08-26 12:14 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-23 19:50 [PATCH v2 00/19] Fix various NPTL synchronization issues Adhemerval Zanella via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 01/19] nptl: Fix tst-cancel7 and tst-cancelx7 race condition (BZ #14232) Adhemerval Zanella via Libc-alpha
2021-08-26  9:33   ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 02/19] nptl: Set cancellation type and state on pthread_exit Adhemerval Zanella via Libc-alpha
2021-08-26  9:38   ` Florian Weimer via Libc-alpha
2021-08-26  9:42     ` Florian Weimer via Libc-alpha
2021-08-26 11:56       ` Adhemerval Zanella via Libc-alpha
2021-08-26 11:52     ` Adhemerval Zanella via Libc-alpha
2021-08-26 12:08       ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 03/19] nptl: Handle robust PI mutexes for !__ASSUME_SET_ROBUST_LIST Adhemerval Zanella via Libc-alpha
2021-08-26  9:42   ` Florian Weimer via Libc-alpha
2021-08-26 12:14     ` Adhemerval Zanella via Libc-alpha [this message]
2021-08-23 19:50 ` [PATCH v2 04/19] nptl: Do not use pthread set_tid_address as state synchronization (BZ #19951) Adhemerval Zanella via Libc-alpha
2021-08-26 10:41   ` Florian Weimer via Libc-alpha
2021-08-26 14:58     ` Adhemerval Zanella via Libc-alpha
2021-08-26 15:06       ` Florian Weimer via Libc-alpha
2021-08-26 16:16         ` Adhemerval Zanella via Libc-alpha
2021-08-30 10:42           ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 05/19] nptl: Move setxid flag out of cancelhandling Adhemerval Zanella via Libc-alpha
2021-08-26 11:34   ` Florian Weimer via Libc-alpha
2021-08-26 15:11     ` Adhemerval Zanella via Libc-alpha
2021-08-26 15:21       ` Florian Weimer via Libc-alpha
2021-08-26 16:39         ` Adhemerval Zanella via Libc-alpha
2021-08-30 11:27           ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 06/19] nptl: Replace struct thread cancelhandling field Adhemerval Zanella via Libc-alpha
2021-08-26 14:34   ` Florian Weimer via Libc-alpha
2021-08-26 16:48     ` Adhemerval Zanella via Libc-alpha
2021-08-30 10:36       ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 07/19] support: Add support_wait_for_thread_exit Adhemerval Zanella via Libc-alpha
2021-08-26  9:31   ` Florian Weimer via Libc-alpha
2021-08-26 16:49     ` Adhemerval Zanella via Libc-alpha
2021-08-30 11:46       ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 08/19] nptl: pthread_kill, pthread_cancel should fail after exit (bug 19193) Adhemerval Zanella via Libc-alpha
2021-08-26 10:03   ` Florian Weimer via Libc-alpha
2021-08-26 16:49     ` Adhemerval Zanella via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 09/19] nptl: Fix race between pthread_kill and thread exit (bug 12889) Adhemerval Zanella via Libc-alpha
2021-08-26 14:23   ` Florian Weimer via Libc-alpha
2021-08-26 17:06     ` Adhemerval Zanella via Libc-alpha
2021-08-30  9:25       ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 10/19] nptl: Use tidlock when accessing TID on pthread_getaffinity_np Adhemerval Zanella via Libc-alpha
2021-08-26 14:24   ` Florian Weimer via Libc-alpha
2021-08-26 17:29     ` Adhemerval Zanella via Libc-alpha
2021-08-30  9:30       ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 11/19] nptl: Use tidlock when accessing TID on pthread_setaffinity Adhemerval Zanella via Libc-alpha
2021-08-26 14:25   ` Florian Weimer via Libc-alpha
2021-08-26 17:31     ` Adhemerval Zanella via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 12/19] nptl: Use tidlock when accessing TID on pthread_getcpuclockid Adhemerval Zanella via Libc-alpha
2021-08-26 14:27   ` Florian Weimer via Libc-alpha
2021-08-26 17:41     ` Adhemerval Zanella via Libc-alpha
2021-08-30  9:34       ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 13/19] nptl: Use tidlock when accessing TID on pthread_getschedparam Adhemerval Zanella via Libc-alpha
2021-08-26 15:00   ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 14/19] nptl: Use tidlock when accessing TID on pthread_setschedparam Adhemerval Zanella via Libc-alpha
2021-08-26 14:35   ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 15/19] nptl: Use tidlock when accessing TID on pthread_getname_np Adhemerval Zanella via Libc-alpha
2021-08-26 14:38   ` Florian Weimer via Libc-alpha
2021-08-26 17:45     ` Adhemerval Zanella via Libc-alpha
2021-08-30  9:37       ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 16/19] nptl: Use tidlock when accessing TID on pthread_setname_np Adhemerval Zanella via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 17/19] nptl: Use tidlock when accessing TID on pthread_sigqueue Adhemerval Zanella via Libc-alpha
2021-08-26 14:43   ` Florian Weimer via Libc-alpha
2021-08-26 17:49     ` Adhemerval Zanella via Libc-alpha
2021-08-30  9:26       ` Florian Weimer via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 18/19] nptl: Use tidlock when accessing TID on pthread_setschedprio Adhemerval Zanella via Libc-alpha
2021-08-23 19:50 ` [PATCH v2 19/19] nptl: Remove INVALID_TD_P Adhemerval Zanella via Libc-alpha
2021-08-26  9:30   ` Florian Weimer via Libc-alpha
2021-08-26 14:47 ` [PATCH v2 00/19] Fix various NPTL synchronization issues Florian Weimer via Libc-alpha
2021-08-26 18:19   ` Adhemerval Zanella via Libc-alpha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6f7d9204-6a81-a021-30b4-e3bbea580fae@linaro.org \
    --to=libc-alpha@sourceware.org \
    --cc=adhemerval.zanella@linaro.org \
    --cc=fweimer@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).