Re: [Xen-devel] [PATCH 2 of 5] xenpaging: map gfn before nomination

["Andres Lagar-Cavilla" <andres@xxxxxxxxxxxxxxxx>]

> On Tue, Dec 06, Andres Lagar-Cavilla wrote:
>
>> Ouch! You're absolutely tying the user space pager with the underlying
>> xen
>> paging code. Most of your patches change the tools and the hypervisor in
>> lockstep.
>
> Yes, pager and hypervisor are bound closely together.
>
>> Patch 4 in your series is one such case. Short-cutting the state
>> machine:
>> great. But what is the gain for the hypervisor in *not* sending the
>> EVICT_FAIL event. It's a good thing. It keeps the same interface to
>> user-space. Xenpaging may not need it, but the Xen paging code does not
>> exist solely for xenpaging.
>
> What IS the need to send yet another request? It adds just overhead for
> no obvious need. Please show the code that will benefit from the extra
> EVICT_FAIL message.

I have mentioned the reasoning in a previous email.

We need to look at the big picture when it comes to overhead. Sending an
event is, what, nanoseconds? -- particularly when guest events are
guaranteed to succeed and not perhaps go to sleep on a wait queue.

The I/O involved is an overhead orders of magnitude much larger. When you
have hundreds (thousands?) of VMs pounding your storage fabric, it becomes
germane to avoid unnecessary I/O.

A pager that performs a batch of nominations, I/O, and evictions, is not
at all unreasonable. It will benefit greatly from early "do not evict"
notices.

The cost/benefit equation here is drastically skewed in favor of one
event, versus one I/O.
>
>> Patch 1 is also a problem. I don't buy that we can number interfaces,
>> and
>> then only support the tip ("Sorry, ENOEXEC, dig out an older
>> hypervisor").
>> Either we bite the bullet of some level of backwards compatibility with
>> all its messiness, or we just keep it in flux until there is convergence
>> on a major "barrier".
>
> An out-of-tree pager can very well try to support a number of
> hypervisors, perhaps patch #1 can be extended to report the age in some
> way. But why would the upstream pager care about (development!) state
> from the past?
>
>
>> This current patch 2 is also rather gratuitous. The need to map before
>> nominate feels superfluous. If Xen cannot deal with nomination requests
>> on
>> pages that have not been mapped, then we've broken Xen.
>
> Why? It was broken (or rather: not optimal) in the first place.
> Any attempt to map a gfn in any  paging state has to return -ENOENT
> right away. The pager is certainly a special case, and thanks to this
> change, and your change for the page-in part, the special handling can now
> get removed.

If you're pushing for this change because you want to get rid from a few
lines of (correct) error checking in the hypervisor, that's not a good
reason imho.

If you have a more compelling reason, sure.

>
>> Anyway, I hope my message gets across. I'm not trying to organize a
>> blockade on your code. I'm trying to get to the best possible hypervisor
>> paging support we can.
>
> Great.
> Then please improve tools/xenpaging/, thats the cure for NIH.

Woaah. Please read my email carefully. This is not about NIH. This about
changes that bring no tangible benefits and break other people's code.

Andres
>
> Olaf
>



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel