Re: [PATCH v15 5/8] arm64: futex: support futex with FEAT_LSUI

[Catalin Marinas <catalin.marinas@arm.com>]

On Fri, Mar 13, 2026 at 09:23:58AM +0000, Yeoreum Yun wrote:
> > On Fri, Feb 27, 2026 at 03:17:02PM +0000, Yeoreum Yun wrote:
> > > +
> > > +		if (__lsui_cmpxchg64(uaddr64, &oval64.raw, nval64.raw))
> > > +			return -EFAULT;
> > > +
> > > +		oldval = oval64.futex[futex_pos];
> > > +		other = oval64.futex[other_pos];
> > > +		orig_other = orig64.futex[other_pos];
> > > +
> > > +		if (other == orig_other) {
> > > +			ret = 0;
> > > +			break;
> > > +		}
> >
> > Is this check correct? What if the cmpxchg64 failed because futex_pos
> > was changed but other_pos remained the same, it will just report success
> > here. You need to compare the full 64-bit value to ensure the cmpxchg64
> > succeeded.
> 
> This is not matter since "futex_cmpxchg_value_locked()" checks
> the "curval" and "oldval" IOW, though it returns success,
> caller of this function always checks the "curval" and "oldval"
> and when it's different, It handles to change return as -EAGAIN.

Ah, ok, it makes sense (I did not check the callers).

> > > +	}
> > > +
> > > +	if (!ret)
> > > +		*oval = oldval;
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static __always_inline int
> > > +__lsui_futex_atomic_and(int oparg, u32 __user *uaddr, int *oval)
> > > +{
> > > +	/*
> > > +	 * Undo the bitwise negation applied to the oparg passed from
> > > +	 * arch_futex_atomic_op_inuser() with FUTEX_OP_ANDN.
> > > +	 */
> > > +	return __lsui_futex_atomic_andnot(~oparg, uaddr, oval);
> > > +}
> > > +
> > > +static __always_inline int
> > > +__lsui_futex_atomic_eor(int oparg, u32 __user *uaddr, int *oval)
> > > +{
> > > +	u32 oldval, newval, val;
> > > +	int ret, i;
> > > +
> > > +	if (get_user(oldval, uaddr))
> > > +		return -EFAULT;
> > > +
> > > +	/*
> > > +	 * there are no ldteor/stteor instructions...
> > > +	 */
> > > +	for (i = 0; i < FUTEX_MAX_LOOPS; i++) {
> > > +		newval = oldval ^ oparg;
> > > +
> > > +		ret = __lsui_cmpxchg32(uaddr, oldval, newval, &val);
> >
> > Since we have a FUTEX_MAX_LOOPS here, do we need it in cmpxchg32 as
> > well?
> >
> > For eor, we need a loop irrespective of whether futex_pos or other_pos
> > have changed. For cmpxchg, we need the loop only if other_pos has
> > changed and return -EAGAIN if futex_pos has changed since the caller
> > needs to update oldval and call again.
> >
> > So try to differentiate these cases, maybe only keep the loop outside
> > cmpxchg32 (I haven't put much though into it).
> 
> I think we can remove loops on __lsui_cmpxchg32() and return -EAGAIN
> when other_pos is different. the __lsui_cmpxchg32() will be called
> "futex_cmpxchg_value_locked()" and as I said, this always checks
> whether curval & oldval when it successed.

Yes, I think for the futex_cmpxchg_value_locked(), the bounded loop
doesn't matter since the core would invoke it back on -EAGAIN. It's nice
not to fail if the actual futex did not change but in practice it
doesn't make any difference and I'd rather keep the code simple.

> But in "eor" when it receive "-EAGAIN" from __lsui_cmxchg32()
> we can simply continue the loop.

Yes, for eor we need the bounded loop. Only return -EAGAIN to the user
if we finished the loop and either __lsui_cmpxchg32() returned -EAGAIN
or the updated on futex_pos failed.

Thanks.

-- 
Catalin