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
