Re: [Qemu-devel] [PATCH 08/10] tcg: add memory barriers in page_find_alloc accesses

[Peter Maydell]

On 12 August 2015 at 17:41, Paolo Bonzini <address@hidden> wrote:
> page_find is reading the radix tree outside all locks, so it has to
> use the RCU primitives.  It does not need RCU critical sections
> because the PageDescs are never removed, so there is never a need
> to wait for the end of code sections that use a PageDesc.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  translate-all.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/translate-all.c b/translate-all.c
> index 7727091..78a787d 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -437,14 +437,14 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
> int alloc)
>
>      /* Level 2..N-1.  */
>      for (i = V_L1_SHIFT / V_L2_BITS - 1; i > 0; i--) {
> -        void **p = *lp;
> +        void **p = atomic_rcu_read(lp);
>
>          if (p == NULL) {
>              if (!alloc) {
>                  return NULL;
>              }
>              p = g_new0(void *, V_L2_SIZE);
> -            *lp = p;
> +            atomic_rcu_set(lp, p);
>          }
>
>          lp = p + ((index >> (i * V_L2_BITS)) & (V_L2_SIZE - 1));
> @@ -456,7 +456,7 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
> int alloc)
>              return NULL;
>          }
>          pd = g_new0(PageDesc, V_L2_SIZE);
> -        *lp = pd;
> +        atomic_rcu_set(lp, pd);
>      }
>
>      return pd + (index & (V_L2_SIZE - 1));

Don't we also need to use an atomic_rcu_read() for the load
    pd = *lp;
(which is between hunk 1 and 2 in this patch) ?

thanks
-- PMM