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. 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 - /* Ultimate fallback: re-execute in serial context. */ cpu_loop_exit_atomic(env_cpu(env), ra); } @@ -186,25 +173,32 @@ static Int128 load_atomic16_or_exit(CPUArchState *env, uintptr_t ra, void *pv) return atomic16_read_ro(p); } -#ifdef CONFIG_USER_ONLY - /* - * We can only use cmpxchg to emulate a load if the page is writable. - * 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(p), 16, PAGE_WRITE_ORG)) { - return *p; - } -#endif - - /* - * In system mode all guest pages are writable, and for user-only - * we have just checked writability. Try cmpxchg. - */ + /* We can only use cmpxchg to emulate a load if the page is writable. */ if (HAVE_ATOMIC128_RW) { +#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. + * We must take mmap_lock so that the query remains valid until + * the write is complete -- tests/tcg/multiarch/munmap-pthread.c + * is an example that can race. + */ + Int128 r; + + mmap_lock(); + if (page_get_flags(h2g(p)) & PAGE_WRITE_ORG) { + r = atomic16_read_rw(p); + } else { + r = *p; + } + mmap_unlock(); + return r; +#else + /* In system mode all guest pages are host writable. */ return atomic16_read_rw(p); +#endif } /* Ultimate fallback: re-execute in serial context. */ -- 2.34.1