On 7/17/23 11:12, Peter Maydell wrote:
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?
It would not be wrong. I was just thinking of the cmpxchg8 part, for which we do not have
a configure probe, and for which I *think* there's no call, because there are no 32-bit
hosts that have cmpxchg8 but not the full CONFIG_ATOMIC64.
r~
