[lttng-dev] [RFC PATCH urcu] Fix: dynamic fallback to compat futex on sys_futex ENOSYS

[Mathieu Desnoyers]

----- On Sep 13, 2015, at 12:29 PM, Paul E. McKenney paulmck at linux.vnet.ibm.com wrote:

> On Fri, Sep 11, 2015 at 10:48:38AM -0400, Mathieu Desnoyers wrote:
>> Some MIPS processors (e.g. Cavium Octeon II) dynamically check if the
>> CPU supports ll/sc within sys_futex, and return a ENOSYS errno if they
>> don't, even though the architecture implements sys_futex.
>> 
>> Handle this situation by always building the sys_futex compatibility
>> layer, and fall-back on it if sys_futex return a ENOSYS errno. This is
>> a tiny compat layer which adds very little space overhead.
>> 
>> This adds an unlikely branch on return from sys_futex, which should
>> not be an issue performance-wise (we've already taken a system call).
>> 
>> Since this is a fall-back mode, don't try to be clever, and don't cache
>> the result, so that the common cases (architectures with a properly
>> working sys_futex) don't get two conditional branches, just one.
> 
> Looks like a reasonable approach to me.
> 
> Acked-by: Paul E. McKenney <paulmck at linux.vnet.ibm.com>

Merged into master, stable-0.8 and stable-0.7,

Thanks!

Mathieu

> 
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
>> CC: Paul E. McKenney <paulmck at linux.vnet.ibm.com>
>> CC: Michael Jeanson <mjeanson at efficios.com>
>> CC: Jon Bernard <jbernard at debian.org>
>> ---
>>  Makefile.am  |  2 --
>>  urcu/futex.h | 70 +++++++++++++++++++++++++++++++++++++++++++++++++-----------
>>  2 files changed, 57 insertions(+), 15 deletions(-)
>> 
>> diff --git a/Makefile.am b/Makefile.am
>> index 752510d..f9a8888 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -41,9 +41,7 @@ else
>>  COMPAT=
>>  endif
>> 
>> -if COMPAT_FUTEX
>>  COMPAT+=compat_futex.c
>> -endif
>> 
>>  RCULFHASH = rculfhash.c rculfhash-mm-order.c rculfhash-mm-chunk.c \
>>  		rculfhash-mm-mmap.c
>> diff --git a/urcu/futex.h b/urcu/futex.h
>> index 2be3bb6..13d2b1a 100644
>> --- a/urcu/futex.h
>> +++ b/urcu/futex.h
>> @@ -47,22 +47,66 @@ extern "C" {
>>   * (returns EINTR).
>>   */
>> 
>> +extern int compat_futex_noasync(int32_t *uaddr, int op, int32_t val,
>> +		const struct timespec *timeout, int32_t *uaddr2, int32_t val3);
>> +extern int compat_futex_async(int32_t *uaddr, int op, int32_t val,
>> +		const struct timespec *timeout, int32_t *uaddr2, int32_t val3);
>> +
>>  #ifdef CONFIG_RCU_HAVE_FUTEX
>> +
>> +#include <unistd.h>
>> +#include <errno.h>
>> +#include <urcu/compiler.h>
>>  #include <urcu/syscall-compat.h>
>> -#define futex(...)	syscall(__NR_futex, __VA_ARGS__)
>> -#define futex_noasync(uaddr, op, val, timeout, uaddr2, val3)	\
>> -		futex(uaddr, op, val, timeout, uaddr2, val3)
>> -#define futex_async(uaddr, op, val, timeout, uaddr2, val3)	\
>> -		futex(uaddr, op, val, timeout, uaddr2, val3)
>> +
>> +static inline int futex(int32_t *uaddr, int op, int32_t val,
>> +		const struct timespec *timeout, int32_t *uaddr2, int32_t val3)
>> +{
>> +	return syscall(__NR_futex, uaddr, op, val, timeout,
>> +			uaddr2, val3);
>> +}
>> +
>> +static inline int futex_noasync(int32_t *uaddr, int op, int32_t val,
>> +		const struct timespec *timeout, int32_t *uaddr2, int32_t val3)
>> +{
>> +	int ret;
>> +
>> +	ret = futex(uaddr, op, val, timeout, uaddr2, val3);
>> +	if (caa_unlikely(ret < 0 && errno == ENOSYS)) {
>> +		return compat_futex_noasync(uaddr, op, val, timeout,
>> +				uaddr2, val3);
>> +	}
>> +	return ret;
>> +
>> +}
>> +
>> +static inline int futex_async(int32_t *uaddr, int op, int32_t val,
>> +		const struct timespec *timeout, int32_t *uaddr2, int32_t val3)
>> +{
>> +	int ret;
>> +
>> +	ret = futex(uaddr, op, val, timeout, uaddr2, val3);
>> +	if (caa_unlikely(ret < 0 && errno == ENOSYS)) {
>> +		return compat_futex_async(uaddr, op, val, timeout,
>> +				uaddr2, val3);
>> +	}
>> +	return ret;
>> +}
>> +
>>  #else
>> -extern int compat_futex_noasync(int32_t *uaddr, int op, int32_t val,
>> -	const struct timespec *timeout, int32_t *uaddr2, int32_t val3);
>> -#define futex_noasync(uaddr, op, val, timeout, uaddr2, val3)	\
>> -		compat_futex_noasync(uaddr, op, val, timeout, uaddr2, val3)
>> -extern int compat_futex_async(int32_t *uaddr, int op, int32_t val,
>> -	const struct timespec *timeout, int32_t *uaddr2, int32_t val3);
>> -#define futex_async(uaddr, op, val, timeout, uaddr2, val3)	\
>> -		compat_futex_async(uaddr, op, val, timeout, uaddr2, val3)
>> +
>> +static inline int futex_noasync(int32_t *uaddr, int op, int32_t val,
>> +		const struct timespec *timeout, int32_t *uaddr2, int32_t val3)
>> +{
>> +	return compat_futex_noasync(uaddr, op, val, timeout, uaddr2, val3);
>> +}
>> +
>> +static inline int futex_async(int32_t *uaddr, int op, int32_t val,
>> +		const struct timespec *timeout, int32_t *uaddr2, int32_t val3)
>> +{
>> +	return compat_futex_async(uaddr, op, val, timeout, uaddr2, val3);
>> +}
>> +
>>  #endif
>> 
>>  #ifdef __cplusplus
>> --
>> 2.1.4

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com