Re: [Xen-devel] [PATCH v4] altp2m: Allow the hostp2m entries to be of type p2m_ram_shared

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 18/07/16 18:06, Tamas K Lengyel wrote:
>>> Anyhow, at this point I'm
>>> going to start carrying out-of-tree patches for Xen in my project and
>>> just resign from my mem_sharing maintainership as I feel like it's
>>> pretty pointless.
>>
>> I'm sorry that you're discouraged; all I can say is that I hope you
>> reconsider.  I'm not trying to block you, and I'm not ignoring your use
>> case; it's the job of a maintainer to look at *everyone's* use cases and
>> try to make sure that they are all accommodated in so far as it is
>> possible.
>>
>> I'm also trying to make sure that the code you end up using in your
>> project is robust and reliable.  It seems to me like if the current
>> implementation was fixed, your life would be a lot easier than if we
>> accept your patch as it is -- your sharing code could just worry about
>> sharing, your altp2m code could just worry about whatever it's trying to
>> do, without having to carefully avoid corner cases or manually fix
>> things up when corner cases happen.  A bit less sharing would happen,
>> because fewer pages would be eligible to be shared, but overall you'd
>> have a lot less bugs and headache.
>>
>> I invested a lot of my very limited time carefully going through both
>> sets of code before I answered your e-mail, and I spent a lot of time
>> trying to explain the kinds of interactions I think will be a problem.
>> I could have just acked the patch without doing that; but I think that
>> would have been both less good for you, and less good for the project as
>> a whole.
> 
> I certainly appreciate your time spent on this. However, I don't see
> the point of being maintainer if my opinion on what constitutes an
> improvement of the system just gets overruled.

You're not being overruled; you're just being asked to make a case for a
change you want to make to an area of code that I maintain (the p2m
code).  And the discussion is by no means over; I started the most
recent discussion by saying "Correct me if I'm wrong", and it looks like
there are still a number of places where we have different views of the
facts of the matter.  Once we've established those we may end up with
closer opinions.

Working together means that sometimes you have to spend the time and
effort to understand where other people are coming from -- what they
think is important, what they think is true; and then working with that
-- correcting them on places where they have misconceptions (or
double-checking your own beliefs to make sure that you're not mistaken);
communicating what it is that you think is important, and then trying to
come up with a way forward that takes everyone's values into account, or
convincing someone that a particular way really is the best way forward
(which may mean convincing them to change their priorities somewhat).

I am sorry that the tone of this conversation has heated up.  But the
reason I've been "raising my voice" as it were is because I've been
trying to ask questions and raise potential issues, and I feel like
you've been just hand-waving them away.  You may be 100% right, but it
is my duty as the maintainer of the p2m code to not accept code until
I'm reasonably convinced that it's a good way forward.

> I would like to hear the
> other maintainers opinion on this matter as well but I'm not
> interested in arguing endlessly or initiating or vote, so if the patch
> is not allowed in I will accept that decision but I will see no point
> in continuing as maintainer of the system.

At a basic level, the other maintainers will agree that I shouldn't
accept code unless I am convinced it's for the good of the project.  But
since this is a technical issue, before anyone would express an opinion
to ask me to change my mind, they would want a more complete view of the
facts of the matter -- facts which you and I are still in the process of
sorting out.

> As pretty much my
> project is the only use-case where these two systems would be used
> together at this point, and since I already require my users to
> compile Xen from source it is just easier to go this route then what
> you suggest and exploring and remedying all possible ways the two
> systems could be misused when setup in ways they were not intended. If
> these were considered stable features and not experimental I would
> agree, but that's just not the case. So I think both of our time is
> better spent doing other things then arguing. 

So a lot of points here.

You have no idea what other projects are doing.  Lots of people take the
Xen code, do something with it internally, and and we never hear from
them.  Or maybe they're in a start-up in stealth mode and will announce
themselves in due course.  If you step down from being a maintainer and
stop engaging with the community you'll be in the same position.

There's a very obvious other use case which I've been talking about from
the beginning: A host administrator / cloud provider / user wants to
both 1) use page sharing to improve memory efficiency, and 2) allow
tools using the altp2m feature to run internally within the guest.
(Given that I have brought it up several times very explicitly, I am in
fact a bit at a loss to understand why you would say your project is the
"only use case where these two systems would be used together".)

Even it were true that your project is the only use case, making the
code so that it only works for your use case is a sure-fire way to
guarantee that there never are any other use cases.

I'm not insisting that you either leave the code broken or make sharing
and altp2m over the same gfn ranges fully robust.  What I've been
suggesting you do is to fix the code so that it works as it is
*intended* to work currently -- that is, that gfns refered to by altp2ms
will be prevented from being shared.  Maybe that's not possible, or
maybe that's not suitable for your use case; but you haven't yet argued
either of those points.

As I've said, it seems to me like the option you've chosen isn't even
the best option *for your project*.  You're giving *yourself* an
interface which is inherently fragile, and for which you as the consumer
of that interface need to think carefully about all the corner cases and
then either avoid them or use mem_sharing to work around them.  Is that
actually more difficult than just doing the work up front to make sure
that the corner cases are handled in a robust manner by Xen?

Finally, yes one of the benefits of being a maintainer is that you can
shepherd and develop the code that you maintain to make sure that it
doesn't degrade and continues to work for use cases that you care about.
 But one of the responsibilities of that is looking not only for your
own use case, but to try to make it a useful tool for the community as a
whole.

Working with the community certainly is a lot of work, but it's a *lot*
less work than writing your own hypervisor.  It's also less work, in the
end, than keeping a massive patchset out-of-tree and having to rebase
every release.  And it often ends up improving and sharpening the code
you wrote anyway.

Peace,
 -George

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