From: Penny Zheng <Penny.Zheng@arm.com>
To: Julien Grall <julien@xen.org>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
"sstabellini@kernel.org" <sstabellini@kernel.org>,
Jan Beulich <jbeulich@suse.com>
Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>,
Wei Chen <Wei.Chen@arm.com>, nd <nd@arm.com>
Subject: RE: [PATCH V4 07/10] xen: re-define assign_pages and introduce assign_page
Date: Tue, 17 Aug 2021 08:21:31 +0000 [thread overview]
Message-ID: <VE1PR08MB5215DCF997C8408A42D6F212F7FE9@VE1PR08MB5215.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <58be6daa-d8d1-1907-c549-585f56075a99@xen.org>
Hi Julian and Jan
> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: Friday, August 13, 2021 8:27 PM
> To: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org;
> sstabellini@kernel.org
> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; Wei Chen
> <Wei.Chen@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH V4 07/10] xen: re-define assign_pages and introduce
> assign_page
>
> Hi Penny,
>
> On 28/07/2021 11:27, 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 assign_page for original page with a single
> order.
> >
> > Backporting confusion could be helped by altering the order of
> > assign_page parameters, such that the compiler would point out that
> > adjustments at call sites are needed.
>
> Looking at the code, you don't alter the order of assign_page() parameters. So
> did you mean to refer to "assign_pages()"?
>
> >
> > Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> > ---
> > v4 change:
> > - in all cases where order-0 pages get passed, prefer using
> > assign_pages to pass literal 1
> > - reconstruct the order of assign_pages parameters
> > - remove the unnecessary parentheses
> > ---
> > xen/arch/x86/pv/dom0_build.c | 2 +-
> > xen/common/grant_table.c | 2 +-
> > xen/common/memory.c | 4 ++--
> > xen/common/page_alloc.c | 23 ++++++++++++++---------
> > 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 af47615b22..9142f359da 100644
> > --- a/xen/arch/x86/pv/dom0_build.c
> > +++ b/xen/arch/x86/pv/dom0_build.c
> > @@ -556,7 +556,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(mfn_to_page(_mfn(mfn++)), 1, d, 0)
> > + )
> > BUG();
> > }
> > initrd->mod_end = 0;
> > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index
> > fab77ab9cc..1f6b89bff4 100644
> > --- a/xen/common/grant_table.c
> > +++ b/xen/common/grant_table.c
> > @@ -2342,7 +2342,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(page, 1, e, MEMF_no_refcount)) )
> > {
> > bool drop_dom_ref;
> >
> > diff --git a/xen/common/memory.c b/xen/common/memory.c index
> > e07bd9a5ea..083e14b84f 100644
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -728,7 +728,7 @@ 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,
> > + if ( assign_page(d, page, exch.out.extent_order,
> > MEMF_no_refcount) )
> > {
> > unsigned long dec_count; @@ -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(page, 1, d, 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
> > f51e406401..e279c6f713 100644
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -2282,9 +2282,9 @@ 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,
> > + struct domain *d,
> > unsigned int memflags)
> > {
> > int rc = 0;
> > @@ -2304,7 +2304,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 ) @@ -2313,18 +2313,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) )
> > {
> > @@ -2336,10 +2336,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);
> > @@ -2354,6 +2354,11 @@ int assign_pages(
> > return rc;
> > }
> >
> > +int assign_page(struct domain *d, struct page_info *pg, unsigned int order,
> > + unsigned int memflags) {
> > + return assign_pages(pg, 1UL << order, d, memflags); }
> >
> > struct page_info *alloc_domheap_pages(
> > struct domain *d, unsigned int order, unsigned int memflags) @@
> > -2396,7 +2401,7 @@ struct page_info *alloc_domheap_pages(
> > pg[i].count_info = PGC_extra;
> > }
> > }
> > - if ( assign_pages(d, pg, order, memflags) )
> > + if ( assign_page(d, pg, order, 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..2e75cdcbb7 100644
> > --- a/xen/include/xen/mm.h
> > +++ b/xen/include/xen/mm.h
> > @@ -132,6 +132,12 @@ int query_page_offline(mfn_t mfn, uint32_t *status);
> > void heap_init_late(void);
> >
> > int assign_pages(
> > + struct page_info *pg,
> > + unsigned long nr,
> > + struct domain *d,
> > + unsigned int memflags);
> > +
> > +int assign_page(
> > struct domain *d,
> > struct page_info *pg,
> > unsigned int order,
>
> I find a bit odd that the parameters are ordered differently between
> assign_pages() and assign_page(). They are similar interface after all.
>
I will change the order back and make them in the similar order.
> I don't think it would be a problem for backporting purpose if
> assign_page() has a different order for the arguments.
>
> Jan, what do you think?
>
> Cheers,
>
> --
> Julien Grall
next prev parent reply other threads:[~2021-08-17 8:22 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-28 10:27 [PATCH V4 00/10] Domain on Static Allocation Penny Zheng
2021-07-28 10:27 ` [PATCH V4 01/10] xen/arm: introduce domain " Penny Zheng
2021-08-11 13:32 ` Julien Grall
2021-08-16 5:21 ` Penny Zheng
2021-08-16 17:27 ` Julien Grall
2021-08-17 2:28 ` Penny Zheng
2021-07-28 10:27 ` [PATCH V4 02/10] xen/arm: introduce new helper device_tree_get_meminfo Penny Zheng
2021-08-11 13:35 ` Julien Grall
2021-08-16 5:27 ` Penny Zheng
2021-07-28 10:27 ` [PATCH V4 03/10] xen/arm: handle static memory in dt_unreserved_regions Penny Zheng
2021-08-11 13:48 ` Julien Grall
2021-08-16 6:00 ` Penny Zheng
2021-08-16 17:33 ` Julien Grall
2021-07-28 10:27 ` [PATCH V4 04/10] xen: introduce mark_page_free Penny Zheng
2021-08-11 14:08 ` Julien Grall
2021-07-28 10:27 ` [PATCH V4 05/10] xen/arm: static memory initialization Penny Zheng
2021-08-04 15:54 ` Jan Beulich
2021-08-13 12:20 ` Julien Grall
2021-08-16 6:12 ` Penny Zheng
2021-08-13 12:38 ` Julien Grall
2021-08-16 7:00 ` Wei Chen
2021-07-28 10:27 ` [PATCH V4 06/10] xen/arm: introduce PGC_reserved Penny Zheng
2021-08-13 12:21 ` Julien Grall
2021-08-16 6:13 ` Penny Zheng
2021-07-28 10:27 ` [PATCH V4 07/10] xen: re-define assign_pages and introduce assign_page Penny Zheng
2021-08-05 6:34 ` Jan Beulich
2021-08-13 12:27 ` Julien Grall
2021-08-13 12:32 ` Jan Beulich
2021-08-17 8:21 ` Penny Zheng [this message]
2021-07-28 10:27 ` [PATCH V4 08/10] xen/arm: introduce acquire_staticmem_pages and acquire_domstatic_pages Penny Zheng
2021-08-05 6:52 ` Jan Beulich
2021-08-13 13:00 ` Julien Grall
2021-08-16 6:43 ` Penny Zheng
2021-08-16 17:43 ` Julien Grall
2021-08-17 2:33 ` Penny Zheng
2021-07-28 10:27 ` [PATCH V4 09/10] xen/arm: check "xen,static-mem" property during domain construction Penny Zheng
2021-08-13 13:12 ` Julien Grall
2021-08-16 6:53 ` Penny Zheng
2021-07-28 10:27 ` [PATCH V4 10/10] xen/arm: introduce allocate_static_memory Penny Zheng
2021-08-13 13:36 ` Julien Grall
2021-08-16 7:51 ` Penny Zheng
2021-08-16 17:55 ` Julien Grall
2021-08-17 2:36 ` Penny Zheng
2021-07-28 10:27 ` [PATCH V4 04/10] xen: introduce mark_page_free Penny Zheng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=VE1PR08MB5215DCF997C8408A42D6F212F7FE9@VE1PR08MB5215.eurprd08.prod.outlook.com \
--to=penny.zheng@arm.com \
--cc=Bertrand.Marquis@arm.com \
--cc=Wei.Chen@arm.com \
--cc=jbeulich@suse.com \
--cc=julien@xen.org \
--cc=nd@arm.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).