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

[Haozhong Zhang <haozhong.zhang@xxxxxxxxx>]

On 12/09/16 17:27 -0500, Konrad Rzeszutek Wilk wrote:
> 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;
> ?
Yes, I forgot this.

Thanks,
Haozhong

> > +}
> > 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