Hi Jan
> -----Original Message-----
> From: Stefano Stabellini <[email protected]>
> Sent: Thursday, September 9, 2021 7:58 AM
> To: Penny Zheng <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> Bertrand Marquis <[email protected]>; Wei Chen
> <[email protected]>; [email protected]
> Subject: Re: [PATCH v6 5/7] xen: re-define assign_pages and introduce a new
> function assign_page
>
> On Wed, 8 Sep 2021, Penny Zheng wrote:
> > In order to deal with the trouble of count-to-order conversion when
> > page number is not in a power-of-two, this commit re-define
> > assign_pages for nr pages and introduces a new helper assign_page for
> original page with a single order.
> >
> > Signed-off-by: Penny Zheng <[email protected]>
>
> The patch looks correct to me, however I don't think I understood Jan's
> request to the previous version of this patch, so I can't tell if you
> addressed his
> concerns.
>
Would you like to take a look at whether I address your concerns in this
version? Thx.
>
> > ---
> > xen/arch/x86/pv/dom0_build.c | 2 +-
> > xen/common/grant_table.c | 2 +-
> > xen/common/memory.c | 6 +++---
> > xen/common/page_alloc.c | 21 +++++++++++++--------
> > xen/include/xen/mm.h | 6 ++++++
> > 5 files changed, 24 insertions(+), 13 deletions(-)
> >
> > diff --git a/xen/arch/x86/pv/dom0_build.c
> > b/xen/arch/x86/pv/dom0_build.c index d7f9e04b28..7787cc8fca 100644
> > --- a/xen/arch/x86/pv/dom0_build.c
> > +++ b/xen/arch/x86/pv/dom0_build.c
> > @@ -568,7 +568,7 @@ int __init dom0_construct_pv(struct domain *d,
> > else
> > {
> > while ( count-- )
> > - if ( assign_pages(d, mfn_to_page(_mfn(mfn++)), 0, 0) )
> > + if ( assign_pages(d, mfn_to_page(_mfn(mfn++)), 1, 0)
> > + )
> > BUG();
> > }
> > initrd->mod_end = 0;
> > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index
> > ee61603a97..50f5f83023 100644
> > --- a/xen/common/grant_table.c
> > +++ b/xen/common/grant_table.c
> > @@ -2358,7 +2358,7 @@ gnttab_transfer(
> > * is respected and speculative execution is blocked accordingly
> > */
> > if ( unlikely(!evaluate_nospec(okay)) ||
> > - unlikely(assign_pages(e, page, 0, MEMF_no_refcount)) )
> > + unlikely(assign_pages(e, page, 1, MEMF_no_refcount)) )
> > {
> > bool drop_dom_ref;
> >
> > diff --git a/xen/common/memory.c b/xen/common/memory.c index
> > 74babb0bd7..9cef8790ff 100644
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -728,8 +728,8 @@ static long
> memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t)
> arg)
> > /* Assign each output page to the domain. */
> > for ( j = 0; (page = page_list_remove_head(&out_chunk_list)); ++j )
> > {
> > - if ( assign_pages(d, page, exch.out.extent_order,
> > - MEMF_no_refcount) )
> > + if ( assign_page(page, exch.out.extent_order, d,
> > + MEMF_no_refcount) )
> > {
> > unsigned long dec_count;
> > bool_t drop_dom_ref;
> > @@ -797,7 +797,7 @@ static long
> memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t)
> arg)
> > * cleared PGC_allocated.
> > */
> > while ( (page = page_list_remove_head(&in_chunk_list)) )
> > - if ( assign_pages(d, page, 0, MEMF_no_refcount) )
> > + if ( assign_pages(d, page, 1, MEMF_no_refcount) )
> > {
> > BUG_ON(!d->is_dying);
> > free_domheap_page(page);
> > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index
> > ba7adc80db..bb19bb10ff 100644
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -2261,7 +2261,7 @@ void init_domheap_pages(paddr_t ps, paddr_t pe)
> > int assign_pages(
> > struct domain *d,
> > struct page_info *pg,
> > - unsigned int order,
> > + unsigned long nr,
> > unsigned int memflags)
> > {
> > int rc = 0;
> > @@ -2281,7 +2281,7 @@ int assign_pages(
> > {
> > unsigned int extra_pages = 0;
> >
> > - for ( i = 0; i < (1ul << order); i++ )
> > + for ( i = 0; i < nr; i++ )
> > {
> > ASSERT(!(pg[i].count_info & ~PGC_extra));
> > if ( pg[i].count_info & PGC_extra ) @@ -2290,18 +2290,18
> > @@ int assign_pages(
> >
> > ASSERT(!extra_pages ||
> > ((memflags & MEMF_no_refcount) &&
> > - extra_pages == 1u << order));
> > + extra_pages == nr));
> > }
> > #endif
> >
> > if ( pg[0].count_info & PGC_extra )
> > {
> > - d->extra_pages += 1u << order;
> > + d->extra_pages += nr;
> > memflags &= ~MEMF_no_refcount;
> > }
> > else if ( !(memflags & MEMF_no_refcount) )
> > {
> > - unsigned int tot_pages = domain_tot_pages(d) + (1 << order);
> > + unsigned int tot_pages = domain_tot_pages(d) + nr;
> >
> > if ( unlikely(tot_pages > d->max_pages) )
> > {
> > @@ -2313,10 +2313,10 @@ int assign_pages(
> > }
> >
> > if ( !(memflags & MEMF_no_refcount) &&
> > - unlikely(domain_adjust_tot_pages(d, 1 << order) == (1 << order)) )
> > + unlikely(domain_adjust_tot_pages(d, nr) == nr) )
> > get_knownalive_domain(d);
> >
> > - for ( i = 0; i < (1 << order); i++ )
> > + for ( i = 0; i < nr; i++ )
> > {
> > ASSERT(page_get_owner(&pg[i]) == NULL);
> > page_set_owner(&pg[i], d);
> > @@ -2331,6 +2331,11 @@ int assign_pages(
> > return rc;
> > }
> >
> > +int assign_page(struct page_info *pg, unsigned int order, struct domain *d,
> > + unsigned int memflags) {
> > + return assign_pages(d, pg, 1UL << order, memflags); }
> >
> > struct page_info *alloc_domheap_pages(
> > struct domain *d, unsigned int order, unsigned int memflags) @@
> > -2373,7 +2378,7 @@ struct page_info *alloc_domheap_pages(
> > pg[i].count_info = PGC_extra;
> > }
> > }
> > - if ( assign_pages(d, pg, order, memflags) )
> > + if ( assign_page(pg, order, d, memflags) )
> > {
> > free_heap_pages(pg, order, memflags & MEMF_no_scrub);
> > return NULL;
> > diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index
> > 8e8fb5a615..a74e93eba8 100644
> > --- a/xen/include/xen/mm.h
> > +++ b/xen/include/xen/mm.h
> > @@ -133,8 +133,14 @@ void heap_init_late(void);
> >
> > int assign_pages(
> > struct domain *d,
> > + struct page_info *pg,
> > + unsigned long nr,
> > + unsigned int memflags);
> > +
> > +int assign_page(
> > struct page_info *pg,
> > unsigned int order,
> > + struct domain *d,
> > unsigned int memflags);
> >
> > /* Dump info to serial console */
> > --
> > 2.25.1
> >
