On Fri, 2026-10-02 at 09:21 +1000, [email protected] wrote:
> From: Alistair Francis <[email protected]>
>
> This reverts commit 5089493e45d3be2bae9ffd2a22186f57b0cabe3f.
>
> Rick reported that this patch results in large slowdowns when using
> multiple threads. The original patch serialises all stores, resulting
> in
> all vCPUs stopping for every store, which would explain the slowdown.
>
> The results from Rick show
>
> xz compressing 2 MB at -6, without and with the patch:
>
> without patch with patch
> xz -T1 5 s 122 s (identical output)
> xz -T8 1 s no output after 600 s
>
> That is too large of an overhead, so let's revert this patch until a
> better solution is found.
>
> Reported-by: Rick <[email protected]>
> Signed-off-by: Alistair Francis <[email protected]>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> linux-user/riscv/target_cpu.h | 2 -
> target/riscv/cpu.h | 3 --
> target/riscv/helper.h | 5 ---
> target/riscv/cpu.c | 3 --
> target/riscv/tcg/op_helper.c | 35 -----------------
> target/riscv/tcg/translate.c | 38 -----------------
> --
> target/riscv/tcg/insn_trans/trans_rva.c.inc | 24 ------------
> target/riscv/tcg/insn_trans/trans_rvi.c.inc | 11 ------
> .../riscv/tcg/insn_trans/trans_rvzawrs.c.inc | 3 --
> 9 files changed, 124 deletions(-)
>
> diff --git a/linux-user/riscv/target_cpu.h b/linux-
> user/riscv/target_cpu.h
> index eff18b37ba..9c642367a3 100644
> --- a/linux-user/riscv/target_cpu.h
> +++ b/linux-user/riscv/target_cpu.h
> @@ -9,8 +9,6 @@ static inline void cpu_clone_regs_child(CPURISCVState
> *env, target_ulong newsp,
> }
>
> env->gpr[xA0] = 0;
> - env->load_res = -1;
> - env->load_res_size = 0;
> }
>
> static inline void cpu_clone_regs_parent(CPURISCVState *env,
> unsigned flags)
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 8c8ffe94cd..c2138dbd4b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -264,9 +264,6 @@ struct CPUArchState {
>
> uint64_t pc;
> uint64_t load_res;
> -#ifdef CONFIG_USER_ONLY
> - uint64_t load_res_size;
> -#endif
> uint64_t load_val;
>
> /* Floating-Point state */
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index aab92bf132..4fc2d3a155 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -1357,8 +1357,3 @@ DEF_HELPER_1(ssamoswap_disabled, void, env)
>
> /* Zalrsc SC write probe */
> DEF_HELPER_FLAGS_3(sc_probe_write, TCG_CALL_NO_WG, void, env, tl,
> tl)
> -
> -#ifdef CONFIG_USER_ONLY
> -/* Invalidate reservations overlapping a completed linux-user store.
> */
> -DEF_HELPER_3(riscv_invalidate_reservations, void, env, tl, tl)
> -#endif
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index da4c1a090b..5fff9d745e 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1084,9 +1084,6 @@ static void riscv_cpu_reset_hold(Object *obj,
> ResetType type)
> env->xl = riscv_cpu_mxl(env);
> cs->exception_index = RISCV_EXCP_NONE;
> env->load_res = -1;
> -#ifdef CONFIG_USER_ONLY
> - env->load_res_size = 0;
> -#endif
> set_default_nan_mode(1, &env->fp_status);
> /* Default NaN value: sign bit clear, frac msb set */
> set_float_default_nan_pattern(0b01000000, &env->fp_status);
> diff --git a/target/riscv/tcg/op_helper.c
> b/target/riscv/tcg/op_helper.c
> index b67c7718db..3e94005d2b 100644
> --- a/target/riscv/tcg/op_helper.c
> +++ b/target/riscv/tcg/op_helper.c
> @@ -19,9 +19,6 @@
> */
>
> #include "qemu/osdep.h"
> -#ifdef CONFIG_USER_ONLY
> -#include "qemu/rcu.h"
> -#endif
> #include "cpu.h"
> #include "target/riscv/tcg/csr.h"
> #ifndef CONFIG_USER_ONLY
> @@ -301,38 +298,6 @@ void helper_sc_probe_write(CPURISCVState *env,
> target_ulong addr,
> probe_write(env, addr, size, mmu_idx, ra);
> }
>
> -#ifdef CONFIG_USER_ONLY
> -
> -void helper_riscv_invalidate_reservations(CPURISCVState *env,
> - target_ulong addr,
> - target_ulong size)
> -{
> - CPUState *cpu;
> -
> - /* EXCP_ATOMIC keeps other guest harts out while this list is
> updated. */
> - WITH_RCU_READ_LOCK_GUARD() {
> - CPU_FOREACH(cpu) {
> - CPURISCVState *other_env = cpu_env(cpu);
> - target_ulong reservation = other_env->load_res;
> - target_ulong reservation_size = other_env-
> >load_res_size;
> - bool overlap;
> -
> - if (other_env == env || reservation == (target_ulong)-1)
> {
> - continue;
> - }
> - overlap = reservation < addr
> - ? addr - reservation < reservation_size
> - : reservation - addr < size;
> - if (overlap) {
> - other_env->load_res = -1;
> - other_env->load_res_size = 0;
> - }
> - }
> - }
> -}
> -
> -#endif
> -
> #ifndef CONFIG_USER_ONLY
>
> target_ulong helper_sret(CPURISCVState *env)
> diff --git a/target/riscv/tcg/translate.c
> b/target/riscv/tcg/translate.c
> index cce3b6dc72..9684dbe752 100644
> --- a/target/riscv/tcg/translate.c
> +++ b/target/riscv/tcg/translate.c
> @@ -42,9 +42,6 @@ static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc;
> static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
> static TCGv_i32 cpu_vl, cpu_vstart;
> static TCGv load_res;
> -#ifdef CONFIG_USER_ONLY
> -static TCGv load_res_size;
> -#endif
> static TCGv load_val;
>
> /*
> @@ -1144,34 +1141,10 @@ static bool gen_unary_per_ol(DisasContext
> *ctx, arg_r2 *a, DisasExtend ext,
> return gen_unary(ctx, a, ext, f_tl);
> }
>
> -#ifdef CONFIG_USER_ONLY
> -static bool gen_riscv_reservation_serialize(DisasContext *ctx)
> -{
> - /* Keep the memory operation and reservation update in one guest
> step. */
> - if (tb_cflags(ctx->base.tb) & CF_PARALLEL) {
> - gen_helper_exit_atomic(tcg_env);
> - ctx->base.is_jmp = DISAS_NORETURN;
> - return false;
> - }
> - return true;
> -}
> -
> -static void gen_riscv_invalidate_reservations(TCGv addr, MemOp mop)
> -{
> - gen_helper_riscv_invalidate_reservations(tcg_env, addr,
> -
> tcg_constant_tl(memop_size(mop)));
> -}
> -#endif
> -
> static bool gen_amo(DisasContext *ctx, arg_atomic *a,
> void(*func)(TCGv, TCGv, TCGv, TCGArg, MemOp),
> MemOp mop)
> {
> -#ifdef CONFIG_USER_ONLY
> - if (!gen_riscv_reservation_serialize(ctx)) {
> - return false;
> - }
> -#endif
> TCGv dest = dest_gpr(ctx, a->rd);
> TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
> MemOp size = mop & MO_SIZE;
> @@ -1186,9 +1159,6 @@ static bool gen_amo(DisasContext *ctx,
> arg_atomic *a,
> decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
> src1 = get_address(ctx, a->rs1, 0);
> func(dest, src1, src2, ctx->mem_idx, mop);
> -#ifdef CONFIG_USER_ONLY
> - gen_riscv_invalidate_reservations(src1, mop);
> -#endif
>
> gen_set_gpr(ctx, a->rd, dest);
> return true;
> @@ -1512,10 +1482,6 @@ void riscv_translate_init(void)
> size_t pc_offset = offsetof(CPURISCVState, pc) +
> field_offset;
> size_t res_offset = offsetof(CPURISCVState, load_res) +
> field_offset;
> size_t val_offset = offsetof(CPURISCVState, load_val) +
> field_offset;
> -#ifdef CONFIG_USER_ONLY
> - size_t res_size_offset = offsetof(CPURISCVState, load_res_size)
> - + field_offset;
> -#endif
>
> for (i = 1; i < 32; i++) {
> cpu_gpr[i] = tcg_global_mem_new(tcg_env,
> @@ -1535,9 +1501,5 @@ void riscv_translate_init(void)
> cpu_vl = tcg_global_mem_new_i32(tcg_env, vl_offset, "vl");
> cpu_vstart = tcg_global_mem_new_i32(tcg_env, vstart_offset,
> "vstart");
> load_res = tcg_global_mem_new(tcg_env, res_offset, "load_res");
> -#ifdef CONFIG_USER_ONLY
> - load_res_size = tcg_global_mem_new(tcg_env, res_size_offset,
> - "load_res_size");
> -#endif
> load_val = tcg_global_mem_new(tcg_env, val_offset, "load_val");
> }
> diff --git a/target/riscv/tcg/insn_trans/trans_rva.c.inc
> b/target/riscv/tcg/insn_trans/trans_rva.c.inc
> index 248a8c60d7..44c1696fe4 100644
> --- a/target/riscv/tcg/insn_trans/trans_rva.c.inc
> +++ b/target/riscv/tcg/insn_trans/trans_rva.c.inc
> @@ -32,11 +32,6 @@
>
> static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
> {
> -#ifdef CONFIG_USER_ONLY
> - if (!gen_riscv_reservation_serialize(ctx)) {
> - return false;
> - }
> -#endif
> TCGv src1;
>
> mop |= MO_ALIGN;
> @@ -58,9 +53,6 @@ static bool gen_lr(DisasContext *ctx, arg_atomic
> *a, MemOp mop)
>
> /* Put addr in load_res, data in load_val. */
> tcg_gen_mov_tl(load_res, src1);
> -#ifdef CONFIG_USER_ONLY
> - tcg_gen_movi_tl(load_res_size, memop_size(mop));
> -#endif
> gen_set_gpr(ctx, a->rd, load_val);
>
> return true;
> @@ -68,17 +60,9 @@ static bool gen_lr(DisasContext *ctx, arg_atomic
> *a, MemOp mop)
>
> static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
> {
> -#ifdef CONFIG_USER_ONLY
> - if (!gen_riscv_reservation_serialize(ctx)) {
> - return false;
> - }
> -#endif
> TCGv dest, src1, src2;
> TCGLabel *l1 = gen_new_label();
> TCGLabel *l2 = gen_new_label();
> -#ifdef CONFIG_USER_ONLY
> - TCGLabel *l3 = gen_new_label();
> -#endif
>
> mop |= MO_ALIGN;
> mop |= ctx->mo_endianness;
> @@ -95,11 +79,6 @@ static bool gen_sc(DisasContext *ctx, arg_atomic
> *a, MemOp mop)
> src2 = get_gpr(ctx, a->rs2, EXT_NONE);
> tcg_gen_atomic_cmpxchg_tl(dest, load_res, load_val, src2,
> ctx->mem_idx, mop);
> -#ifdef CONFIG_USER_ONLY
> - tcg_gen_brcond_tl(TCG_COND_NE, dest, load_val, l3);
> - gen_riscv_invalidate_reservations(src1, mop);
> - gen_set_label(l3);
> -#endif
> tcg_gen_setcond_tl(TCG_COND_NE, dest, dest, load_val);
> gen_set_gpr(ctx, a->rd, dest);
> tcg_gen_br(l2);
> @@ -125,9 +104,6 @@ static bool gen_sc(DisasContext *ctx, arg_atomic
> *a, MemOp mop)
> * an SC to any address, in between an LR and SC pair.
> */
> tcg_gen_movi_tl(load_res, -1);
> -#ifdef CONFIG_USER_ONLY
> - tcg_gen_movi_tl(load_res_size, 0);
> -#endif
>
> return true;
> }
> diff --git a/target/riscv/tcg/insn_trans/trans_rvi.c.inc
> b/target/riscv/tcg/insn_trans/trans_rvi.c.inc
> index 40b4c9aa4a..cc1b5dbbad 100644
> --- a/target/riscv/tcg/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/tcg/insn_trans/trans_rvi.c.inc
> @@ -489,9 +489,6 @@ static bool gen_store_tl(DisasContext *ctx,
> arg_sb *a, MemOp memop)
> }
>
> tcg_gen_qemu_st_tl(data, addr, ctx->mem_idx, memop);
> -#ifdef CONFIG_USER_ONLY
> - gen_riscv_invalidate_reservations(addr, memop);
> -#endif
> return true;
> }
>
> @@ -521,19 +518,11 @@ static bool gen_store_i128(DisasContext *ctx,
> arg_sb *a, MemOp memop)
> }
> tcg_gen_qemu_st_i128(t16, addrl, ctx->mem_idx, memop);
> }
> -#ifdef CONFIG_USER_ONLY
> - gen_riscv_invalidate_reservations(addrl, memop);
> -#endif
> return true;
> }
>
> static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop)
> {
> -#ifdef CONFIG_USER_ONLY
> - if (!gen_riscv_reservation_serialize(ctx)) {
> - return false;
> - }
> -#endif
> memop |= ctx->mo_endianness;
> if (ctx->cfg_ptr->ext_zama16b) {
> memop |= MO_ATOM_WITHIN16;
> diff --git a/target/riscv/tcg/insn_trans/trans_rvzawrs.c.inc
> b/target/riscv/tcg/insn_trans/trans_rvzawrs.c.inc
> index 1831bbcfc0..0eef033838 100644
> --- a/target/riscv/tcg/insn_trans/trans_rvzawrs.c.inc
> +++ b/target/riscv/tcg/insn_trans/trans_rvzawrs.c.inc
> @@ -32,9 +32,6 @@ static bool trans_wrs_sto(DisasContext *ctx,
> arg_wrs_sto *a)
>
> /* Clear the load reservation (if any). */
> tcg_gen_movi_tl(load_res, -1);
> -#ifdef CONFIG_USER_ONLY
> - tcg_gen_movi_tl(load_res_size, 0);
> -#endif
>
> gen_update_pc(ctx, ctx->cur_insn_len);
> tcg_gen_exit_tb(NULL, 0);
