Re: [Xen-devel] [PATCH v3 1/2] x86/mm: factor out the code for shattering an l3 PTE

[Jan Beulich <jbeulich@suse.com>]

On 11.12.2019 17:27, Xia, Hongyan wrote:
> On Wed, 2019-12-11 at 16:32 +0100, Jan Beulich wrote:
>> On 11.12.2019 11:58, Hongyan Xia wrote:
>>> @@ -5578,27 +5597,8 @@ int modify_xen_mappings(unsigned long s,
>>> unsigned long e, unsigned int nf)
>>>              }
>>>  
>>>              /* PAGE1GB: shatter the superpage and fall through. */
>>> -            pl2e = alloc_xen_pagetable();
>>> -            if ( !pl2e )
>>> +            if ( shatter_l3e(pl3e, 0, locking) )
>>>                  return -ENOMEM;
>>
>> Hmm, I didn't expect I'd need to comment on this again: As per
>> my v2 reply, you should hand on the return value from the
>> function, not make up your own. This is so that in case the
>> function gains another error path with a different error code,
>> it wouldn't become indistinguishable to callers further up.
> 
> I was basically thinking about the conversation we had that ENOMEM is
> probably the only error value map_pages_to_xen would return ever, and
> it is unlikely to gain another return value in the future, so initially
> I just let shatter return -1 and the caller return -ENOMEM. There is no
> problem for me if we want to change it to handle different error
> values.

The alternative to your prior 0 / -1 returning would have been to
have the function return bool. In this case "inventing" an error
code here would be fine. The 0 / -1 approach would introduce
another instance of what we're trying to get rid of elsewhere.

Jan

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