[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [PATCH v2 4/9] xen/arm: introduce put_page_nr and get_page_nr
On Sat, 7 May 2022, Penny Zheng wrote: > > On Fri, 6 May 2022, Penny Zheng wrote: > > > Later, we need to add the right amount of references, which should be > > > the number of borrower domains, to the owner domain. Since we only > > > have > > > get_page() to increment the page reference by 1, a loop is needed per > > > page, which is inefficient and time-consuming. > > > > > > To save the loop time, this commit introduces a set of new helpers > > > put_page_nr() and get_page_nr() to increment/drop the page reference by > > nr. > > > > > > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx> > > > --- > > > v2 change: > > > - new commit > > > --- > > > xen/arch/arm/include/asm/mm.h | 4 ++++ > > > xen/arch/arm/mm.c | 36 +++++++++++++++++++++++++---------- > > > 2 files changed, 30 insertions(+), 10 deletions(-) > > > > > > diff --git a/xen/arch/arm/include/asm/mm.h > > > b/xen/arch/arm/include/asm/mm.h index 424aaf2823..c737d51e4d 100644 > > > --- a/xen/arch/arm/include/asm/mm.h > > > +++ b/xen/arch/arm/include/asm/mm.h > > > @@ -347,6 +347,10 @@ void free_init_memory(void); int > > > guest_physmap_mark_populate_on_demand(struct domain *d, unsigned > > long gfn, > > > unsigned int order); > > > > > > +extern bool get_page_nr(struct page_info *page, const struct domain > > *domain, > > > + unsigned long nr); extern void > > > +put_page_nr(struct page_info *page, unsigned long nr); > > > + > > > extern void put_page_type(struct page_info *page); static inline > > > void put_page_and_type(struct page_info *page) { diff --git > > > a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 7b1f2f4906..e565979f3c > > > 100644 > > > --- a/xen/arch/arm/mm.c > > > +++ b/xen/arch/arm/mm.c > > > @@ -1537,7 +1537,8 @@ long arch_memory_op(int op, > > XEN_GUEST_HANDLE_PARAM(void) arg) > > > return 0; > > > } > > > > > > -struct domain *page_get_owner_and_reference(struct page_info *page) > > > +static struct domain *page_get_owner_and_nr_reference(struct page_info > > *page, > > > + unsigned long > > > +nr) > > > { > > > unsigned long x, y = page->count_info; > > > struct domain *owner; > > > @@ -1545,13 +1546,14 @@ struct domain > > *page_get_owner_and_reference(struct page_info *page) > > > do { > > > x = y; > > > /* > > > + * Consider the minimum case(nr = 1): > > > * Count == 0: Page is not allocated, so we cannot take a > > > reference. > > > * Count == -1: Reference count would wrap, which is invalid. > > > */ > > > if ( unlikely(((x + 1) & PGC_count_mask) <= 1) ) > > > return NULL; > > > } > > > - while ( (y = cmpxchg(&page->count_info, x, x + 1)) != x ); > > > + while ( (y = cmpxchg(&page->count_info, x, x + nr)) != x ); > > > > > > owner = page_get_owner(page); > > > ASSERT(owner); > > > @@ -1559,36 +1561,50 @@ struct domain > > *page_get_owner_and_reference(struct page_info *page) > > > return owner; > > > } > > > > > > -void put_page(struct page_info *page) > > > +struct domain *page_get_owner_and_reference(struct page_info *page) { > > > + return page_get_owner_and_nr_reference(page, 1); } > > > + > > > +void put_page_nr(struct page_info *page, unsigned long nr) > > > { > > > unsigned long nx, x, y = page->count_info; > > > > > > do { > > > - ASSERT((y & PGC_count_mask) != 0); > > > + ASSERT(((y - nr) & PGC_count_mask) >= 0); > > > > Why this change? The original ASSERT is to check that we enter the loop only > > when count_info is greater than 0. It should still apply even for > > put_page_nr > > without modifications? > > > > > > Oh, I understand wrongly about the ASSERT. I thought it was to > check the result count_info after the loop will be not smaller than 0. > > Do you think we still need to ensure that? Maybe ASSERT( ((y & > PGC_count_mask) != 0) && (((y - nr) & PGC_count_mask) >= 0)); ? I think it should be: ASSERT((y & PGC_count_mask) >= nr);
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |