| Message ID | 20161010003235.4213-6-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > --- > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > --- > 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@lists.xen.org > https://lists.xen.org/xen-devel
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@intel.com> >> --- >> Cc: Jan Beulich <jbeulich@suse.com> >> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> 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@lists.xen.org >> https://lists.xen.org/xen-devel
>>> On 10.10.16 at 02:32, <haozhong.zhang@intel.com> wrote: > --- 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; > + 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; Why are these needed? > + return pmem_teardown_preemptible(d, &preempted); I don't see what you have "preempted" for here. You want the function to return -ERESTART in the preemption case. And I also don't see what the helper function is good for - the code can easily live right in this function. Jan
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; + /* 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; + 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); +} 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__ */
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@intel.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/domain.c | 5 +++++ xen/arch/x86/pmem.c | 41 +++++++++++++++++++++++++++++++++++++++++ xen/include/xen/pmem.h | 1 + 3 files changed, 47 insertions(+)