Re: [patch 3/6] futex: Get rid of the val2 conditional dance

[André Almeida]

Hi Thomas,

Às 16:44 de 22/04/21, Thomas Gleixner escreveu:
> There is no point in checking which FUTEX operand treats the utime pointer
> as 'val2' argument because that argument to do_futex() is only used by
> exactly these operands.
>
> So just handing it in unconditionally is not making any difference, but
> removes a lot of pointless gunk.
>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> ---
>   kernel/futex.c |   16 ++--------------
>   1 file changed, 2 insertions(+), 14 deletions(-)
>
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -3765,7 +3765,6 @@ SYSCALL_DEFINE6(futex, u32 __user *, uad
>   {
>   	struct timespec64 ts;
>   	ktime_t t, *tp = NULL;
> -	u32 val2 = 0;
>   	int cmd = op & FUTEX_CMD_MASK;
>   
>   	if (utime && (cmd == FUTEX_WAIT || cmd == FUTEX_LOCK_PI ||
> @@ -3785,15 +3784,8 @@ SYSCALL_DEFINE6(futex, u32 __user *, uad
>   			t = timens_ktime_to_host(CLOCK_MONOTONIC, t);
>   		tp = &t;
>   	}
> -	/*
> -	 * requeue parameter in 'utime' if cmd == FUTEX_*_REQUEUE_*.
> -	 * number of waiters to wake in 'utime' if cmd == FUTEX_WAKE_OP.
> -	 */
> -	if (cmd == FUTEX_REQUEUE || cmd == FUTEX_CMP_REQUEUE ||
> -	    cmd == FUTEX_CMP_REQUEUE_PI || cmd == FUTEX_WAKE_OP)
> -		val2 = (u32) (unsigned long) utime;
>   
> -	return do_futex(uaddr, op, val, tp, uaddr2, val2, val3);
> +	return do_futex(uaddr, op, val, tp, uaddr2, (unsigned long)utime, val3);

Given do_futex()'s type signature, I think it makes more sense to cast 
utime to u32.

>   }
>   
>   #ifdef CONFIG_COMPAT
> @@ -3961,7 +3953,6 @@ SYSCALL_DEFINE6(futex_time32, u32 __user
>   {
>   	struct timespec64 ts;
>   	ktime_t t, *tp = NULL;
> -	int val2 = 0;
>   	int cmd = op & FUTEX_CMD_MASK;
>   
>   	if (utime && (cmd == FUTEX_WAIT || cmd == FUTEX_LOCK_PI ||
> @@ -3979,11 +3970,8 @@ SYSCALL_DEFINE6(futex_time32, u32 __user
>   			t = timens_ktime_to_host(CLOCK_MONOTONIC, t);
>   		tp = &t;
>   	}
> -	if (cmd == FUTEX_REQUEUE || cmd == FUTEX_CMP_REQUEUE ||
> -	    cmd == FUTEX_CMP_REQUEUE_PI || cmd == FUTEX_WAKE_OP)
> -		val2 = (int) (unsigned long) utime;
>   
> -	return do_futex(uaddr, op, val, tp, uaddr2, val2, val3);
> +	return do_futex(uaddr, op, val, tp, uaddr2, (unsigned long)utime, val3);

Same here.

>   }
>   #endif /* CONFIG_COMPAT_32BIT_TIME */
>