From patchwork Tue May 31 16:40:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 12654 Received: (qmail 63444 invoked by alias); 31 May 2016 16:40:42 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 63422 invoked by uid 89); 31 May 2016 16:40:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=concrete, Hx-languages-length:5311, H*F:D*cc, died X-HELO: Chamillionaire.breakpoint.cc Date: Tue, 31 May 2016 18:40:25 +0200 From: Sebastian Andrzej Siewior To: Torvald Riegel Cc: tglx@linutronix.de, Roland McGrath , libc-alpha@sourceware.org, carlos@redhat.com Subject: [PATCH RFC][BZ 19944] ntpl: pthread_mutex_lock() use a wrapper for error checking Message-ID: <20160531164025.GA19018@breakpoint.cc> References: <1460576781-12017-1-git-send-email-bigeasy@linutronix.de> <20160413201514.B86212C3A9B@topped-with-meat.com> <20160413205202.GA12533@breakpoint.cc> <1460644570.3869.423.camel@localhost.localdomain> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1460644570.3869.423.camel@localhost.localdomain> User-Agent: Mutt/1.6.0 (2016-04-01) Implement a wrapper around kernel's FUTEX_LOCK_PI opcode / syscall to filter error codes. Returned to caller: - EDEADLOCK: detected a deadlock - EINVAL: if uaddr's address is not a multiple of four. - ENOMEM: failed to allocate memory for PI state. I am not sure if it is better to return it to the caller or retry again. A sleep() between invocation is probably a bad idea if the process is RT. Trying again is probably a bad idea if the process is RT with the highest priority. - EAGAIN: The owner is about to terminate. Same as with ENOMEM (not sure what is the best action here). - ESRCH: Happens also for non-robust futex if the owner exits. The lock value becomes FUTEX_OWNER_DIED set. Returned as EOWNERDEAD. Error codes which should not happen invoke futex_fatal_error(). 2016-04-13 Sebastian Andrzej Siewior [BZ #19944] (partly) * sysdeps/nptl/futex-internal.h: futex_lock_pi () prototype * sysdeps/unix/sysv/linux/futex-internal.h: implement futex_lock_pi() * nptl/pthread_mutex_lock.c: use new wrapper futex_lock_pi () --- Compile tested only. Could we extend futex_fatal_error() a little to pass the opcode + error code? If it is not fully reproducible it would be nice to know what led to it. nptl/pthread_mutex_lock.c | 31 ++++++--------------------- sysdeps/nptl/futex-internal.h | 3 +++ sysdeps/unix/sysv/linux/futex-internal.h | 36 ++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c index bdfa529f639b..4ecd2d8a91ec 100644 --- a/nptl/pthread_mutex_lock.c +++ b/nptl/pthread_mutex_lock.c @@ -26,6 +26,7 @@ #include #include #include +#include #ifndef lll_lock_elision #define lll_lock_elision(lock, try_lock, private) ({ \ @@ -332,36 +333,16 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex) { /* The mutex is locked. The kernel will now take care of everything. */ - int private = (robust - ? PTHREAD_ROBUST_MUTEX_PSHARED (mutex) - : PTHREAD_MUTEX_PSHARED (mutex)); - INTERNAL_SYSCALL_DECL (__err); - int e = INTERNAL_SYSCALL (futex, __err, 4, &mutex->__data.__lock, - __lll_private_flag (FUTEX_LOCK_PI, - private), 1, 0); + int ret; - if (INTERNAL_SYSCALL_ERROR_P (e, __err) - && (INTERNAL_SYSCALL_ERRNO (e, __err) == ESRCH - || INTERNAL_SYSCALL_ERRNO (e, __err) == EDEADLK)) - { - assert (INTERNAL_SYSCALL_ERRNO (e, __err) != EDEADLK - || (kind != PTHREAD_MUTEX_ERRORCHECK_NP - && kind != PTHREAD_MUTEX_RECURSIVE_NP)); - /* ESRCH can happen only for non-robust PI mutexes where - the owner of the lock died. */ - assert (INTERNAL_SYSCALL_ERRNO (e, __err) != ESRCH || !robust); - - /* Delay the thread indefinitely. */ - while (1) - pause_not_cancel (); - } + ret = futex_lock_pi(mutex); + if (ret) + return ret; oldval = mutex->__data.__lock; - - assert (robust || (oldval & FUTEX_OWNER_DIED) == 0); } - if (__glibc_unlikely (oldval & FUTEX_OWNER_DIED)) + if (__glibc_unlikely (oldval & FUTEX_OWNER_DIED) && robust) { atomic_and (&mutex->__data.__lock, ~FUTEX_OWNER_DIED); diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h index d798b6970893..3c376788437a 100644 --- a/sysdeps/nptl/futex-internal.h +++ b/sysdeps/nptl/futex-internal.h @@ -192,6 +192,9 @@ futex_abstimed_wait_cancelable (unsigned int* futex_word, static __always_inline void futex_wake (unsigned int* futex_word, int processes_to_wake, int private); +/* Invoke kernel's FUTEX_LOCK_PI syscall. */ +static __always_inline int futex_lock_pi(pthread_mutex_t *mutex); + /* Calls __libc_fatal with an error message. Convenience function for concrete implementations of the futex interface. */ static __always_inline __attribute__ ((__noreturn__)) void diff --git a/sysdeps/unix/sysv/linux/futex-internal.h b/sysdeps/unix/sysv/linux/futex-internal.h index 1add836ebc2f..6b571ae82e33 100644 --- a/sysdeps/unix/sysv/linux/futex-internal.h +++ b/sysdeps/unix/sysv/linux/futex-internal.h @@ -248,4 +248,40 @@ futex_wake (unsigned int *futex_word, int processes_to_wake, int private) } } +static __always_inline int futex_lock_pi(pthread_mutex_t *mutex) +{ + INTERNAL_SYSCALL_DECL (err); + int robust = mutex->__data.__kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP; + int private; + int ret; + + private = (robust + ? PTHREAD_ROBUST_MUTEX_PSHARED (mutex) + : PTHREAD_MUTEX_PSHARED (mutex)); + + ret = INTERNAL_SYSCALL (futex, err, 4, &mutex->__data.__lock, + __lll_private_flag (FUTEX_LOCK_PI, + private), 1, 0); + if (!INTERNAL_SYSCALL_ERROR_P(ret, err)) + return 0; + ret = INTERNAL_SYSCALL_ERRNO (ret, err); + switch (ret) + { + case EDEADLOCK: /* dead lock detected */ + case EINVAL: /* the __lock variable is not properly aligned */ + case ENOMEM: /* internal memory allocation failed */ + case EAGAIN: /* lock owner is terminating */ + return ret; + case ESRCH: /* The process holding the lock is gone */ + return EOWNERDEAD; + default: + case EPERM: /* not possible (unlock) or PID of owner belongs to a + kernel thread */ + case EFAULT: /* we dereferenced the pointer, kernel should be able to + do, too */ + case ETIMEDOUT: /* didn't ask for timeout */ + futex_fatal_error (); + } +} + #endif /* futex-internal.h */