Re: [Xen-devel] [PATCH] simplify xenmem_add_to_physmap_batch()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/12/17 16:21, Jan Beulich wrote:
> There's no need for
> - advancing the handles and at the same time using
>   __copy_{from,to}_guest_offset(),
> - an "out" label,
> - local variables "done" and (function scope) "rc".
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

These changes do mean that start is no longer ideally named.  How about
s/start/extent/, which also matches up better with the parameter name
passed in by its caller?

~Andrew

>
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -820,65 +820,37 @@ static int xenmem_add_to_physmap_batch(s
>                                         struct xen_add_to_physmap_batch 
> *xatpb,
>                                         unsigned int start)
>  {
> -    unsigned int done = 0;
> -    int rc;
> -
>      if ( xatpb->size < start )
>          return -EILSEQ;
>  
> -    guest_handle_add_offset(xatpb->idxs, start);
> -    guest_handle_add_offset(xatpb->gpfns, start);
> -    guest_handle_add_offset(xatpb->errs, start);
> -    xatpb->size -= start;
> -
> -    if ( !guest_handle_okay(xatpb->idxs, xatpb->size) ||
> -         !guest_handle_okay(xatpb->gpfns, xatpb->size) ||
> -         !guest_handle_okay(xatpb->errs, xatpb->size) )
> +    if ( !guest_handle_subrange_okay(xatpb->idxs, start, xatpb->size - 1) ||
> +         !guest_handle_subrange_okay(xatpb->gpfns, start, xatpb->size - 1) ||
> +         !guest_handle_subrange_okay(xatpb->errs, start, xatpb->size - 1) )
>          return -EFAULT;
>  
> -    while ( xatpb->size > done )
> +    while ( xatpb->size > start )
>      {
>          xen_ulong_t idx;
>          xen_pfn_t gpfn;
> +        int rc;
>  
> -        if ( unlikely(__copy_from_guest_offset(&idx, xatpb->idxs, 0, 1)) )
> -        {
> -            rc = -EFAULT;
> -            goto out;
> -        }
> -
> -        if ( unlikely(__copy_from_guest_offset(&gpfn, xatpb->gpfns, 0, 1)) )
> -        {
> -            rc = -EFAULT;
> -            goto out;
> -        }
> +        if ( unlikely(__copy_from_guest_offset(&idx, xatpb->idxs, start, 1)) 
> ||
> +             unlikely(__copy_from_guest_offset(&gpfn, xatpb->gpfns, start, 
> 1)) )
> +            return -EFAULT;
>  
>          rc = xenmem_add_to_physmap_one(d, xatpb->space,
>                                         xatpb->u,
>                                         idx, _gfn(gpfn));
>  
> -        if ( unlikely(__copy_to_guest_offset(xatpb->errs, 0, &rc, 1)) )
> -        {
> -            rc = -EFAULT;
> -            goto out;
> -        }
> -
> -        guest_handle_add_offset(xatpb->idxs, 1);
> -        guest_handle_add_offset(xatpb->gpfns, 1);
> -        guest_handle_add_offset(xatpb->errs, 1);
> +        if ( unlikely(__copy_to_guest_offset(xatpb->errs, start, &rc, 1)) )
> +            return -EFAULT;
>  
>          /* Check for continuation if it's not the last iteration. */
> -        if ( xatpb->size > ++done && hypercall_preempt_check() )
> -        {
> -            rc = start + done;
> -            goto out;
> -        }
> +        if ( xatpb->size > ++start && hypercall_preempt_check() )
> +            return start;
>      }
>  
> -    rc = 0;
> -
> -out:
> -    return rc;
> +    return 0;
>  }
>  
>  static int construct_memop_from_reservation(
>
>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel