Re: [Xen-devel] [PATCH v3 04/15] argo: init, destroy and soft-reset, with enable command line opt

[Roger Pau Monné <royger@xxxxxxxxxxx>]

On Mon, Jan 14, 2019 at 3:48 PM Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
>
> Hi all
>
> The locking scheme seems to be remaining sticking point. The rest are
> mostly cosmetic issues (FAOD, they still need to be addressed).  Frankly
> I don't think there is enough time to address all the technical details,
> but let me sum up each side's position and see if we can reach an
> amicable solution.
>
> From maintainers and reviewers' point of view:
>
> 1. Maintainers / reviewers don't like complexity unless absolutely
>    necessary.
> 2. Maintainers / reviewers feel they have a responsibility to understand
>    the code and algorithm.
>
> Yet being the gatekeepers doesn't necessarily mean we understand every
> technical details and every usecase. We would like to, but most of the
> time it is unrealistic.
>
> Down to this specific patch series:
>
> Roger thinks the locking scheme is too complex. Christopher argues
> that's necessary for short-live channels to be performant.
>
> Both have their point.
>
> I think having a complex locking scheme is inevitable, just like we did
> for performant grant table several years ago.  Regardless of the timing
> issue we have at hand, asking Christopher to implement a stripped down
> version creates more work for him.
>
> Yet ignoring Roger's concerns is unfair to him as well, since he put in
> so much time and effort to understand the algorithm and provide
> suggestions. It is in fact unreasonable to ask anyone to fully
> understand the locking mechanism and check the implementation is correct
> in a few days (given the series was posted in Dec and there were major
> holidays in between, plus everyone had other commitments).
>
> To unblock this, how about we make Christopher maintainer of Argo? He
> and OpenXT will be on the hook for further improvement. And I believe it
> would be in their best interest to keep Argo bug-free and eventually
> make it become supported.
>
> So:
>
> 1. Make sure Argo is self-contained -- this requires careful review for
>    interaction between Argo and other parts of the hypervisor.
> 2. Argo is going to be experimental and off-by-default -- this is the
>    default status for new feature anyway.
> 3. Make Christopher maintainer of Argo -- this would be a natural thing
>    to do anyway.
>
> Does this work for everyone?

I think this is fine.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel