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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.05.2020 17:45, Roger Pau Monné wrote:
> 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>

Thanks.

>> --- 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.

Oh, indeed, good point.

Jan