On Sun, 16 Jul 2023 at 18:03, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> For user-only, the probe for page writability may race with another
> thread's mprotect. Take the mmap_lock around the operation. This
> is still faster than the start/end_exclusive fallback.
>
> Remove the write probe in load_atomic8_or_exit. There we don't have
> the same machinery for testing the existance of an 8-byte cmpxchg.
"existence"
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> accel/tcg/ldst_atomicity.c.inc | 54 +++++++++++++++-------------------
> 1 file changed, 24 insertions(+), 30 deletions(-)
>
> diff --git a/accel/tcg/ldst_atomicity.c.inc b/accel/tcg/ldst_atomicity.c.inc
> index 4de0a80492..e7170f8ba2 100644
> --- a/accel/tcg/ldst_atomicity.c.inc
> +++ b/accel/tcg/ldst_atomicity.c.inc
> @@ -152,19 +152,6 @@ static uint64_t load_atomic8_or_exit(CPUArchState *env,
> uintptr_t ra, void *pv)
> return load_atomic8(pv);
> }
>
> -#ifdef CONFIG_USER_ONLY
> - /*
> - * If the page is not writable, then assume the value is immutable
> - * and requires no locking. This ignores the case of MAP_SHARED with
> - * another process, because the fallback start_exclusive solution
> - * provides no protection across processes.
> - */
> - if (page_check_range(h2g(pv), 8, PAGE_WRITE_ORG)) {
> - uint64_t *p = __builtin_assume_aligned(pv, 8);
> - return *p;
> - }
> -#endif
I don't really understand the comment in the commit message:
why would it be wrong to wrap this "test writeability and
do the operation" in the mmap-lock, the same way we do for the
16-byte case?
thanks
-- PMM
