Re: [PATCH v3 7/8] accel/tcg: Move PageDesc tree into tb-maint.c for system

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 9/12/22 06:19, Richard Henderson wrote:
> Now that PageDesc is not used for user-only, and for system
> it is only used for tb maintenance, move the implementation
> into tb-main.c appropriately ifdefed.
> 
> We have not yet eliminated all references to PageDesc for
> user-only, so retain a typedef to the structure without definition.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/internal.h      |  49 +++-----------
>   accel/tcg/tb-maint.c      | 130 ++++++++++++++++++++++++++++++++++++--
>   accel/tcg/translate-all.c |  95 ----------------------------
>   3 files changed, 134 insertions(+), 140 deletions(-)


> diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
> index 20e86c813d..9b996bbeb2 100644
> --- a/accel/tcg/tb-maint.c
> +++ b/accel/tcg/tb-maint.c

Expanding diff context:

     #ifdef CONFIG_USER_ONLY

     ...

>   #else


> +PageDesc *page_find_alloc(tb_page_addr_t index, bool alloc)
> +{
> +    PageDesc *pd;
> +    void **lp;
> +    int i;
> +
> +    /* Level 1.  Always allocated.  */
> +    lp = l1_map + ((index >> v_l1_shift) & (v_l1_size - 1));
> +
> +    /* Level 2..N-1.  */
> +    for (i = v_l2_levels; i > 0; i--) {
> +        void **p = qatomic_rcu_read(lp);
> +
> +        if (p == NULL) {
> +            void *existing;
> +
> +            if (!alloc) {
> +                return NULL;
> +            }
> +            p = g_new0(void *, V_L2_SIZE);
> +            existing = qatomic_cmpxchg(lp, NULL, p);
> +            if (unlikely(existing)) {
> +                g_free(p);
> +                p = existing;
> +            }
> +        }
> +
> +        lp = p + ((index >> (i * V_L2_BITS)) & (V_L2_SIZE - 1));
> +    }
> +
> +    pd = qatomic_rcu_read(lp);
> +    if (pd == NULL) {
> +        void *existing;

            int i;

> +
> +        if (!alloc) {
> +            return NULL;
> +        }
> +        pd = g_new0(PageDesc, V_L2_SIZE);
> +#ifndef CONFIG_USER_ONLY

CONFIG_USER_ONLY never defined here, so this block can be simplified.

> +        {
> +            int i;
> +
> +            for (i = 0; i < V_L2_SIZE; i++) {
> +                qemu_spin_init(&pd[i].lock);
> +            }
> +        }
> +#endif
> +        existing = qatomic_cmpxchg(lp, NULL, pd);
> +        if (unlikely(existing)) {
> +#ifndef CONFIG_USER_ONLY
> +            {

Ditto.

> +                int i;
> +
> +                for (i = 0; i < V_L2_SIZE; i++) {
> +                    qemu_spin_destroy(&pd[i].lock);
> +                }
> +            }
> +#endif
> +            g_free(pd);
> +            pd = existing;
> +        }
> +    }
> +
> +    return pd + (index & (V_L2_SIZE - 1));
> +}