[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.