Re: [Xen-devel] [PATCH v6 01/23] xen: factor out construct_memop_from_reservation

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/02/15 15:55, Wei Liu wrote:
> No functional change.
>
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> Cc: Jan Beulich <JBeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
>  xen/common/memory.c | 52 +++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index e84ace9..d24b001 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -692,11 +692,43 @@ out:
>      return rc;
>  }
>  
> +static int construct_memop_from_reservation(
> +               const struct xen_memory_reservation *r,
> +               struct memop_args *a)
> +{
> +    int rc;
> +    unsigned int address_bits;
> +
> +    a->extent_list  = r->extent_start;
> +    a->nr_extents   = r->nr_extents;
> +    a->extent_order = r->extent_order;
> +    a->memflags     = 0;
> +
> +    address_bits = XENMEMF_get_address_bits(r->mem_flags);
> +    if ( (address_bits != 0) &&
> +         (address_bits < (get_order_from_pages(max_page) + PAGE_SHIFT)) )
> +    {
> +        if ( address_bits <= PAGE_SHIFT )
> +        {
> +            rc = -EINVAL;
> +            goto out;
> +        }
> +        a->memflags = MEMF_bits(address_bits);
> +    }
> +
> +    a->memflags |= MEMF_node(XENMEMF_get_node(r->mem_flags));
> +    if ( r->mem_flags & XENMEMF_exact_node_request )
> +        a->memflags |= MEMF_exact_node;
> +
> +    rc = 0;
> + out:
> +    return rc;
> +}
> +
>  long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
>      struct domain *d;
>      long rc;
> -    unsigned int address_bits;
>      struct xen_memory_reservation reservation;
>      struct memop_args args;
>      domid_t domid;
> @@ -718,25 +750,11 @@ long do_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>          if ( unlikely(start_extent >= reservation.nr_extents) )
>              return start_extent;
>  
> -        args.extent_list  = reservation.extent_start;
> -        args.nr_extents   = reservation.nr_extents;
> -        args.extent_order = reservation.extent_order;
>          args.nr_done      = start_extent;
>          args.preempted    = 0;

As construct_memop_from_reservation() nominally has free reign over all
fields in args, these two assignments should move after the call.

Other than that, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

> -        args.memflags     = 0;
>  
> -        address_bits = XENMEMF_get_address_bits(reservation.mem_flags);
> -        if ( (address_bits != 0) &&
> -             (address_bits < (get_order_from_pages(max_page) + PAGE_SHIFT)) )
> -        {
> -            if ( address_bits <= PAGE_SHIFT )
> -                return start_extent;
> -            args.memflags = MEMF_bits(address_bits);
> -        }
> -
> -        args.memflags |= MEMF_node(XENMEMF_get_node(reservation.mem_flags));
> -        if ( reservation.mem_flags & XENMEMF_exact_node_request )
> -            args.memflags |= MEMF_exact_node;
> +        if ( construct_memop_from_reservation(&reservation, &args) )
> +            return start_extent;
>  
>          if ( op == XENMEM_populate_physmap
>               && (reservation.mem_flags & XENMEMF_populate_on_demand) )



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