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

Heiko Carstens
2022-10-21T19:23:12Z

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").