Re: [PATCH 13/14] xen/gnttab: Simplify gnttab_map_frame()

[Julien Grall <julien.grall@arm.com>]

Hi Andrew,

On 11/21/18 1:21 PM, Andrew Cooper wrote:
>   * Reflow some lines to remove unnecessary line breaks.
>   * Factor out the gnttab_get_frame_gfn() calculation.  Neither x86 nor ARM
>     builds seem to be able to fold the two calls, and the resulting code is far
>     easier to follow.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> ---
>   xen/common/grant_table.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index d5a1a00..f00eba9 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -3869,8 +3869,7 @@ static int gnttab_get_shared_frame_mfn(struct domain *d,
>       return 0;
>   }
>   
> -int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
> -                     mfn_t *mfn)
> +int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn, mfn_t *mfn)
>   {
>       int rc = 0;
>       struct grant_table *gt = d->grant_table;
> @@ -3878,8 +3877,7 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
>   
>       grant_write_lock(gt);
>   
> -    if ( gt->gt_version == 2 &&
> -         (idx & XENMAPIDX_grant_table_status) )
> +    if ( gt->gt_version == 2 && (idx & XENMAPIDX_grant_table_status) )
>       {
>           idx &= ~XENMAPIDX_grant_table_status;
>           status = true;
> @@ -3889,10 +3887,13 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
>       else
>           rc = gnttab_get_shared_frame_mfn(d, idx, mfn);
>   
> -    if ( !rc && paging_mode_translate(d) &&
> -         !gfn_eq(gnttab_get_frame_gfn(gt, status, idx), INVALID_GFN) )
> -        rc = guest_physmap_remove_page(d, gnttab_get_frame_gfn(gt, status, idx),
> -                                       *mfn, 0);
> +    if ( !rc && paging_mode_translate(d) )
> +    {
> +        gfn_t gfn = gnttab_get_frame_gfn(gt, status, idx);
> +
> +        if ( !gfn_eq(gfn, INVALID_GFN) )
> +            rc = guest_physmap_remove_page(d, gfn, *mfn, 0);
> +    }
>   
>       if ( !rc )
>           gnttab_set_frame_gfn(gt, status, idx, gfn);
> 

-- 
Julien Grall

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