Re: [Xen-devel] [PATCH v2 03/30] xen/x86: fix parameters and return value of *_set_allocation functions

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Mon, Oct 03, 2016 at 09:05:43AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of
> > Roger Pau Monne
> > Sent: 27 September 2016 16:57
> > To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: George Dunlap <George.Dunlap@xxxxxxxxxx>; Andrew Cooper
> > <Andrew.Cooper3@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>; Jan Beulich
> > <jbeulich@xxxxxxxx>; boris.ostrovsky@xxxxxxxxxx; Roger Pau Monne
> > <roger.pau@xxxxxxxxxx>
> > Subject: [Xen-devel] [PATCH v2 03/30] xen/x86: fix parameters and return
> > value of *_set_allocation functions
> > 
> > Return should be an int, and the number of pages should be an unsigned
> > long.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> > Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: Tim Deegan <tim@xxxxxxx>
> > ---
> >  xen/arch/x86/mm/hap/hap.c       |  6 +++---
> >  xen/arch/x86/mm/shadow/common.c |  7 +++----
> >  xen/include/asm-x86/domain.h    | 12 ++++++------
> >  3 files changed, 12 insertions(+), 13 deletions(-)
> > 
> > diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> > index 3218fa2..b6d2c61 100644
> > --- a/xen/arch/x86/mm/hap/hap.c
> > +++ b/xen/arch/x86/mm/hap/hap.c
> > @@ -325,7 +325,7 @@ static void hap_free_p2m_page(struct domain *d,
> > struct page_info *pg)
> >  static unsigned int
> >  hap_get_allocation(struct domain *d)
> >  {
> > -    unsigned int pg = d->arch.paging.hap.total_pages
> > +    unsigned long pg = d->arch.paging.hap.total_pages
> >          + d->arch.paging.hap.p2m_pages;
> 
> You are modifying this calculation (by including hap.p2m_pages as well as 
> hap.total_pages) so the comment should probably mention this.

I think your mail reader is fooling you, the '+' was already there before my 
change (and in fact I haven't even touched that line).
 
> > 
> >      return ((pg >> (20 - PAGE_SHIFT))
> > @@ -334,8 +334,8 @@ hap_get_allocation(struct domain *d)
> > 
> >  /* Set the pool of pages to the required number of pages.
> >   * Returns 0 for success, non-zero for failure. */
> > -static unsigned int
> > -hap_set_allocation(struct domain *d, unsigned int pages, int *preempted)
> > +static int
> > +hap_set_allocation(struct domain *d, unsigned long pages, int
> > *preempted)
> >  {
> >      struct page_info *pg;
> > 
> > diff --git a/xen/arch/x86/mm/shadow/common.c
> > b/xen/arch/x86/mm/shadow/common.c
> > index 21607bf..d3cc2cc 100644
> > --- a/xen/arch/x86/mm/shadow/common.c
> > +++ b/xen/arch/x86/mm/shadow/common.c
> > @@ -1613,9 +1613,8 @@ shadow_free_p2m_page(struct domain *d, struct
> > page_info *pg)
> >   * Input will be rounded up to at least shadow_min_acceptable_pages(),
> >   * plus space for the p2m table.
> >   * Returns 0 for success, non-zero for failure. */
> > -static unsigned int sh_set_allocation(struct domain *d,
> > -                                      unsigned int pages,
> > -                                      int *preempted)
> > +static int sh_set_allocation(struct domain *d, unsigned long pages,
> > +                             int *preempted)
> >  {
> >      struct page_info *sp;
> >      unsigned int lower_bound;
> > @@ -1692,7 +1691,7 @@ static unsigned int sh_set_allocation(struct domain
> > *d,
> >  /* Return the size of the shadow pool, rounded up to the nearest MB */
> >  static unsigned int shadow_get_allocation(struct domain *d)
> >  {
> > -    unsigned int pg = d->arch.paging.shadow.total_pages
> > +    unsigned long pg = d->arch.paging.shadow.total_pages
> >          + d->arch.paging.shadow.p2m_pages;
> 
> Same here.
> 
> >      return ((pg >> (20 - PAGE_SHIFT))
> >              + ((pg & ((1 << (20 - PAGE_SHIFT)) - 1)) ? 1 : 0));
> 
> OMG. Is there no rounding macro you can use for this?

Hm, I don't think there's any, and the code was already there (I haven't 
added this), so I will just leave it as-is.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel