Re: [PATCH v2 1/6] x86/mem-paging: fold p2m_mem_paging_prep()'s main if()-s

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Apr 23, 2020 at 10:37:06AM +0200, Jan Beulich wrote:
> The condition of the second can be true only if the condition of the
> first was met; the second half of the condition of the second then also
> is redundant with an earlier check. Combine them, drop a pointless
> local variable, and re-flow the affected gdprintk().
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> 
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1808,6 +1808,8 @@ int p2m_mem_paging_prep(struct domain *d
>      /* Allocate a page if the gfn does not have one yet */
>      if ( !mfn_valid(mfn) )
>      {
> +        void *guest_map;
> +
>          /* If the user did not provide a buffer, we disallow */
>          ret = -EINVAL;
>          if ( unlikely(user_ptr == NULL) )
> @@ -1819,22 +1821,16 @@ int p2m_mem_paging_prep(struct domain *d
>              goto out;
>          mfn = page_to_mfn(page);
>          page_extant = 0;
> -    }
> -
> -    /* If we were given a buffer, now is the time to use it */
> -    if ( !page_extant && user_ptr )
> -    {
> -        void *guest_map;
> -        int rc;
>  
>          ASSERT( mfn_valid(mfn) );

I would be tempted to remove this assert also, since you just
successfully allocated the page at this point.

Thanks, Roger.