Re: [PATCH v12 6/7] arm64: futex: support futex with FEAT_LSUI

[Yeoreum Yun <yeoreum.yun@arm.com>]

Hi Catalin,

Thanks for your suggestion.

> I wonder whether we can shorten this function a bit. Not sure it would
> be more readable but it would be shorter.
>
> On Wed, Jan 21, 2026 at 07:06:21PM +0000, Yeoreum Yun wrote:
> > +static __always_inline int
> > +__lsui_cmpxchg32(u32 __user *uaddr, u32 oldval, u32 newval, u32 *oval)
> > +{
> > +	u64 __user *uaddr64;
> > +	bool futex_on_lo;
> > +	int ret, i;
> > +	u32 other, orig_other;
> > +	union {
> > +		struct futex_on_lo {
> > +			u32 val;
> > +			u32 other;
> > +		} lo_futex;
> > +
> > +		struct futex_on_hi {
> > +			u32 other;
> > +			u32 val;
> > +		} hi_futex;
> > +
> > +		u64 raw;
> > +	} oval64, orig64, nval64;
>
> 	union {
> 		u32 futex[2];
> 		u64 raw;
> 	}
>
> > +
> > +	uaddr64 = (u64 __user *) PTR_ALIGN_DOWN(uaddr, sizeof(u64));
> > +	futex_on_lo = IS_ALIGNED((unsigned long)uaddr, sizeof(u64));
>
> 	futex_pos = (unsigned long)uaddr & 4 ? 1 : 0;

Okay. I'll try.

>
> > +
> > +	if (futex_on_lo) {
> > +		oval64.lo_futex.val = oldval;
> > +		ret = get_user(oval64.lo_futex.other, uaddr + 1);
> > +	} else {
> > +		oval64.hi_futex.val = oldval;
> > +		ret = get_user(oval64.hi_futex.other, uaddr - 1);
> > +	}
>
> and here use
>
> 	get_user(oval64.raw, uaddr64);
> 	futex[futex_pos] = oldval;

But there is another feedback about this
(though I did first similarly with your suggestion -- use oval64.raw):
  https://lore.kernel.org/all/aXDZGhFQDvoSwdc_@willie-the-truck/

>
> > +
> > +	if (ret)
> > +		return -EFAULT;
> > +
> > +	ret = -EAGAIN;
> > +	for (i = 0; i < FUTEX_MAX_LOOPS; i++) {
> > +		orig64.raw = nval64.raw = oval64.raw;
> > +
> > +		if (futex_on_lo)
> > +			nval64.lo_futex.val = newval;
> > +		else
> > +			nval64.hi_futex.val = newval;
> > +
> > +		if (__lsui_cmpxchg64(uaddr64, &oval64.raw, nval64.raw))
> > +			return -EFAULT;
> > +
> > +		if (futex_on_lo) {
> > +			oldval = oval64.lo_futex.val;
> > +			other = oval64.lo_futex.other;
> > +			orig_other = orig64.lo_futex.other;
> > +		} else {
> > +			oldval = oval64.hi_futex.val;
> > +			other = oval64.hi_futex.other;
> > +			orig_other = orig64.hi_futex.other;
> > +		}
>
> Something similar here to use futex[futex_pos].
>
> We probably also need to check that the user pointer is 32-bit aligned
> and return -EFAULT if not.

Thanks. I'll respin it again ;)

--
Sincerely,
Yeoreum Yun