Re: [Qemu-devel] [PATCH v1 1/3] cputlb: ensure tbl_set_dirty1 updates addr_write atomically

[Richard Henderson]

On 03/21/2017 01:34 AM, Alex Bennée wrote:
> This was an oversight when the rest of cputlb was being updated. As
> before it falls back to the non-atomic version when the host can't
> support wider-than-bus atomics.
>
> Signed-off-by: Alex Bennée <address@hidden>
> ---
>  cputlb.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/cputlb.c b/cputlb.c
> index f5d056cc08..0d52e45dfd 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -540,9 +540,17 @@ void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, 
> ram_addr_t length)
>
>  static inline void tlb_set_dirty1(CPUTLBEntry *tlb_entry, target_ulong vaddr)
>  {
> +#if TCG_OVERSIZED_GUEST
>      if (tlb_entry->addr_write == (vaddr | TLB_NOTDIRTY)) {
>          tlb_entry->addr_write = vaddr;
>      }
> +#else
> +    uintptr_t orig_addr = atomic_mb_read(&tlb_entry->addr_write);
> +
> +    if (orig_addr == (vaddr | TLB_NOTDIRTY)) {
> +        atomic_cmpxchg(&tlb_entry->addr_write, orig_addr, vaddr);
> +    }

What's the state machine here?  How can the per-cpu tlb change in a way other 
than dirty->clean / clean->dirty?  AFAIK, we shouldn't be swapping out the tlb 
entry to somthing completely different.

So how does cmpxchg improve over atomic_write?


r~