Re: [Xen-devel] [PATCH V6 5/5] xen: Handle resumed instruction based on previous mem_event reply

[George Dunlap <dunlapg@xxxxxxxxx>]

On Wed, Sep 10, 2014 at 10:28 PM, Razvan Cojocaru
<rcojocaru@xxxxxxxxxxxxxxx> wrote:
> As far as I understand, George is asking about why events that have
> npfec.kind != npfec_kind_with_gla are being emulated instead of being
> sent out like the rest, and if that's a requirement that all memory
> introspection clients might have.
>
> To answer that question, _our_ application is not interested in events
> other than npfec_kind_with_gla, and because of that it seemed worthwhile
> to save a HV <-> dom0 roundtrip for events that would need to be ignored
> by the application anyway, and thus keep the guest as responsive as
> possible. I can't, of course, state that no other introspection client
> will be interested in the other types of events.  But I can add another
> parameter to xc_mem_access_enable_introspection() (please see patch 3/5
> in the series) to specify whether non-npfec_kind_with_gla events should
> be ignored or not (is this what the ..._EMULATE_SILENT suggestion refers
> to?).

I know when you come new to the list it seems like we're being a bunch
of old stodgy skeptics nitpicking everything for no good reason.  :-)

So just to explain a bit where we're coming from: We're always happy
to have improvements from people, and we're glad to have more people
using Xen.  But there are interfaces that we're going to have to be
supporting long-term.  When you're writing a product and you own the
entire piece of code, you can make all these kinds of changes however
you want; the only people it will affect in the future are you.
Adding them to a shared project like Xen, they affect lots of other
people who aren't doing what you're doing.   Furthermore, every
addition to the interface makes it a tiny bit more complicated,
fragile, and easy to break; and it's fairly likely that we'll end up
being the ones having to fix it at some point.

So we definitely want you to be able to write your introspection
engine and have it run well.  But we also want to make sure that you
don't break anything for anyone else; and, we want to make sure that
anything you're adding to the interface is worth the extra cost in
terms of maintenance: that means pushing back and asking if you could
accomplish your goals just as well with the existing interface, or
with a simpler interface that is 1) useful generally, not just to you,
and 2) simplier and easier for us to maintain.

And we realize you may not have experience with this kind of project
before, which is why we're giving you feedback (even if we sometimes
we forget what it's like to be new and get annoyed at having to say
the same thing to every new person who wants to contribute -- you'll
have to cut us some slack too).

So with that in mind:

This "don't give a notification on npfec_kind_with_gla" is a separate
feature that should have been introduced in a separate patch.  Of
course it's nice not to have to do context switches when you don't
have to; but adding a whole raft of flags for events you want to be
able to skip opens up a can of worms interface wise.  So it needs to
be justified. How expensive is it actually to just have the controller
ignore these?  How many unwanted events like this do you get on a
regular basis, and does it actually have a measurable performance
impact?

And if it does have a measurable performance impact, is it likely that
we're going to have other events that we may want to be able to filter
out in the hypervisor?  Is it better to think about a more general /
scalable way of specifying these events, rather than adding flags in
an ad-hoc fashion as they come up?

On the whole, if you're hoping to have the less controversial bits
(patches 1-3, and the other half of this patch) in 4.5, you might want
to set this to the side and come back to it after the code freeze is
done.

 -George

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