Re: [Xen-devel] [PATCH v7] x86/p2m: use large pages for MMIO mappings

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.02.16 at 13:17, <george.dunlap@xxxxxxxxxx> wrote:
> On 09/02/16 11:48, Jan Beulich wrote:
>>>>> On 09.02.16 at 11:56, <george.dunlap@xxxxxxxxxx> wrote:
>>> On 09/02/16 08:42, Jan Beulich wrote:
>>>>>>> On 08.02.16 at 19:04, <george.dunlap@xxxxxxxxxx> wrote:
>>>>> First -- and this isn't a blocker, but just a question (and sorry if
>>>>> it's been answered before) -- why have the "0 means I did it all, <nr
>>>>> means I did it partially"?  Why not just return the number of entries
>>>>> actually handled?
>>>>
>>>> Because I view zero as the conventional "success" indicator. No
>>>> other deeper reason.
>>>
>>> For certain classes of functionality, yes.  But for other classes --
>>> where the caller will request N but the OS / hypervisor / whatever may
>>> do some other number fewer than N, then the convention is to always pass
>>> the number of items completed.  read() and write() system calls are like
>>> this, as are most of the XENMEM operations (e.g.,
>>> XENMEM_increase_resevration).
>>>
>>> Since this domctl looks a lot like some of those XENMEM operations,
>>> wouldn't it make more sense to follow that convention, and return the
>>> number of extents actually allocated?
>> 
>> Well, if comparing with the XENMEM ones, perhaps. But if comparing
>> with other domctls, perhaps rather not? I've really been undecided
>> here from the very beginning, since I had considered that alternative
>> right away.
> 
> Well from a brief survey of things that can partially succeed
> (getmemlist, gethvmcontext, {get,set}_vcpu_msrs), it seems that most of
> them:
> 
> 1. Use the return value *only* for success or failure; they either
> return 0 or -errno, even if they only partially succeed
> 2. Return the actual number of things accomplished in the struct itself.
> 
> You seem to be introducing a new model, where you use the return value
> sort of for all three.  (Are there any other hypercalls that behave this
> way?)
> 
> I don't think sometimes returning the number of things you did and
> sometimes returning zero makes any sense.  My suggestion would be either
> make "nr_mfns" bidirectional (as similar fields are in the other
> domctls) and return 0 on either full or partial success, or just return
> the number of mfns actually mapped either on full or partial success.

As said - I can see your point, and I've been considering the
alternatives and had to decide for one. Since I've already got
Ian's approval for the currently implementation, and since we're
at v7 and I've already spent way more time on this than I had
expected, I hope you understand that I'm a little hesitant to
make more changes (perhaps even requiring re-obtaining acks,
which has by itself been taking long enough for this patch) than
absolutely necessary to get this in.

So - Ian, do you think the alternative proposed by George
would make for a meaningfully better interface?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel