Re: [PATCH v2 1/9] s390/uaccess: Add storage key checked cmpxchg access to user space

[Heiko Carstens]

On Thu, Oct 20, 2022 at 03:40:56PM +0200, Nico Boehr wrote:
> Quoting Janis Schoetterl-Glausch (2022-10-12 22:56:01)
> > +               "2:     lr      %[old_word],%[tmp]\n"
> > +               "3:     cs      %[tmp],%[new_word],%[aligned]\n"
> > +               "4:     jnl     5f\n"
> > +               /* We'll restore old_word before the cs, use reg for the diff */
> > +               "       xr      %[old_word],%[tmp]\n"
> > +               /* Apply diff assuming only bits outside target byte(s) changed */
> > +               "       xr      %[new_word],%[old_word]\n"
> > +               /* If prior assumption false we exit loop, so not an issue */
> > +               "       nr      %[old_word],%[mask]\n"
> > +               "       jz      2b\n"
> 
> So if the remainder changed but the actual value to exchange stays the same, we
> loop in the kernel. Does it maybe make sense to limit the number of iterations
> we spend retrying? I think while looping here the calling process can't be
> killed, can it?

Yes, the number of loops should be limited; quite similar what arm64
implemented with commit 03110a5cb216 ("arm64: futex: Bound number of
LDXR/STXR loops in FUTEX_WAKE_OP").