Re: [PATCH v3 4/4] x86_64/mm: map and unmap page tables in destroy_m2p_mapping

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.04.2020 13:59, Hongyan Xia wrote:
> @@ -285,26 +286,30 @@ static void destroy_m2p_mapping(struct mem_hotadd_info 
> *info)
>              continue;
>          }
>  
> -        l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
> -        if (!(l2e_get_flags(l2_ro_mpt[l2_table_offset(va)]) & _PAGE_PRESENT))
> +        pl2e = map_l2t_from_l3e(l3_ro_mpt[l3_table_offset(va)]) +
> +                    l2_table_offset(va);
> +        if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
>          {
>              i = ( i & ~((1UL << (L2_PAGETABLE_SHIFT - 3)) - 1)) +
>                      (1UL << (L2_PAGETABLE_SHIFT - 3)) ;
> +            UNMAP_DOMAIN_PAGE(pl2e);
>              continue;
>          }
>  
> -        pt_pfn = l2e_get_pfn(l2_ro_mpt[l2_table_offset(va)]);
> +        pt_pfn = l2e_get_pfn(*pl2e);
>          if ( hotadd_mem_valid(pt_pfn, info) )
>          {
>              destroy_xen_mappings(rwva, rwva + (1UL << L2_PAGETABLE_SHIFT));
>  
> -            l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
> -            l2e_write(&l2_ro_mpt[l2_table_offset(va)], l2e_empty());
> +            l2e_write(pl2e, l2e_empty());
>          }
>          i = ( i & ~((1UL << (L2_PAGETABLE_SHIFT - 3)) - 1)) +
>                (1UL << (L2_PAGETABLE_SHIFT - 3));
> +        UNMAP_DOMAIN_PAGE(pl2e);

Along the lines of comments given elsewhere I would have expected
this one to change to the lower case version, as it again sits
right at the and of the scope of the variable.

>      }
>  
> +    UNMAP_DOMAIN_PAGE(l3_ro_mpt);

This, otoh, is still a few lines away from its end-of-scope, and
hence I can see why the variable clearing variant is being used.

Jan