Re: [Xen-devel] [RFC XEN PATCH 05/16] xen/x86: release pmem pages at domain destroy

[Konrad Rzeszutek Wilk <konrad@xxxxxxxxxx>]

On Mon, Oct 10, 2016 at 08:32:24AM +0800, Haozhong Zhang wrote:
> The host pmem pages mapped to a domain are unassigned at domain destroy
> so as to be used by other domains in future.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
>  xen/arch/x86/domain.c  |  5 +++++
>  xen/arch/x86/pmem.c    | 41 +++++++++++++++++++++++++++++++++++++++++
>  xen/include/xen/pmem.h |  1 +
>  3 files changed, 47 insertions(+)
> 
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 1bd5eb6..05ab389 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -61,6 +61,7 @@
>  #include <asm/amd.h>
>  #include <xen/numa.h>
>  #include <xen/iommu.h>
> +#include <xen/pmem.h>
>  #include <compat/vcpu.h>
>  #include <asm/psr.h>
>  
> @@ -2512,6 +2513,10 @@ int domain_relinquish_resources(struct domain *d)
>          if ( ret )
>              return ret;
>  
> +        ret = pmem_teardown(d);
> +        if ( ret )
> +            return ret;

Good, so if ret == -ERESTART it preempts, but..
> +
>          /* Tear down paging-assistance stuff. */
>          ret = paging_teardown(d);
>          if ( ret )
> diff --git a/xen/arch/x86/pmem.c b/xen/arch/x86/pmem.c
> index e4dc685..50e496b 100644
> --- a/xen/arch/x86/pmem.c
> +++ b/xen/arch/x86/pmem.c
> @@ -282,3 +282,44 @@ int pmem_populate(struct xen_pmemmap_args *args)
>      args->nr_done = i;
>      return rc;
>  }
> +
> +static int pmem_teardown_preemptible(struct domain *d, int *preempted)
> +{
> +    struct page_info *pg, *next;
> +    int rc = 0;
> +
> +    spin_lock(&d->pmem_lock);
> +
> +    page_list_for_each_safe (pg, next, &d->pmem_page_list )
> +    {
> +        BUG_ON(page_get_owner(pg) != d);
> +        BUG_ON(page_state_is(pg, free));
> +
> +        page_list_del(pg, &d->pmem_page_list);
> +        page_set_owner(pg, NULL);
> +        pg->count_info = (pg->count_info & ~PGC_count_mask) | PGC_state_free;
> +
> +        if ( preempted && hypercall_preempt_check() )
> +        {
> +            *preempted = 1;

.. you don't set rc = -ERSTART ?

> +            goto out;
> +        }
> +    }
> +
> + out:
> +    spin_unlock(&d->pmem_lock);
> +    return rc;
> +}
> +
> +int pmem_teardown(struct domain *d)
> +{
> +    int preempted = 0;
> +
> +    ASSERT(d->is_dying);
> +    ASSERT(d != current->domain);
> +
> +    if ( !has_hvm_container_domain(d) || !paging_mode_translate(d) )
> +        return -EINVAL;
> +
> +    return pmem_teardown_preemptible(d, &preempted);

Not exactly sure what the 'preempted' is for? You don't seem to be
using it here?

Perhaps you meant to do:

  rc = pmem_teardown_preemptible(d, &preempted);
  if ( preempted )
    return -ERESTART;
  return rc;
?

> +}
> diff --git a/xen/include/xen/pmem.h b/xen/include/xen/pmem.h
> index 60adf56..ffbef1c 100644
> --- a/xen/include/xen/pmem.h
> +++ b/xen/include/xen/pmem.h
> @@ -37,5 +37,6 @@ int pmem_add(unsigned long spfn, unsigned long epfn,
>               unsigned long rsv_spfn, unsigned long rsv_epfn,
>               unsigned long data_spfn, unsigned long data_epfn);
>  int pmem_populate(struct xen_pmemmap_args *args);
> +int pmem_teardown(struct domain *d);
>  
>  #endif /* __XEN_PMEM_H__ */
> -- 
> 2.10.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel

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