Re: [Xen-devel] [PATCH v2] xen: prevent PVH Dom0 from having pages with more than one ref

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/06/14 12:43, Roger Pau Monne wrote:
> With the current implementation of the Dom0 domain builder a PVH Dom0
> ends up with pages that have 2 references, which means they are not
> freed when Dom0 balloons memory down. This leads to those pages
> not being freed in guest_remove_page, so they are leaked and the
> memory accounting between Xen and Dom0 differs, because Dom0 OS
> actually thinks they are freed, but d->tot_pages is not decreased.

Please can you explain the cause of the problem, and state that it is a
difference between PV and HVM memory management.

>
> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> Cc: Keir Fraser <keir@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> ---
> Changes since v1:
>  - Fix coding style.
>  - Fix a second occurrence of the same issue.
>  - Use paging_mode_translate instead of is_pvh_domain.
> ---
>  xen/arch/x86/domain_build.c |   18 +++++++++++-------
>  1 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
> index ba42fc9..842ed5f 100644
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -1137,10 +1137,13 @@ int __init construct_dom0(
>                                      L1_PROT : COMPAT_L1_PROT));
>          l1tab++;
>  
> -        page = mfn_to_page(mfn);
> -        if ( (page->u.inuse.type_info == 0) &&
> -             !get_page_and_type(page, d, PGT_writable_page) )
> -            BUG();
> +        if ( !paging_mode_translate(d) )
> +        {
> +            page = mfn_to_page(mfn);
> +            if ( (page->u.inuse.type_info == 0) &&
> +                 !get_page_and_type(page, d, PGT_writable_page) )
> +                BUG();
> +        }
>      }
>  
>      if ( is_pv_32on64_domain(d) )
> @@ -1300,9 +1303,10 @@ int __init construct_dom0(
>          if ( get_gpfn_from_mfn(mfn) >= count )
>          {
>              BUG_ON(is_pv_32bit_domain(d));
> -            if ( !page->u.inuse.type_info &&
> -                 !get_page_and_type(page, d, PGT_writable_page) )
> -                BUG();
> +            if ( !paging_mode_translate(d) )
> +                if ( !page->u.inuse.type_info &&
> +                     !get_page_and_type(page, d, PGT_writable_page) )
> +                    BUG();
>  

These two ifs should be joined, which looks as if it will reduce the
delta to 1 line.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel