Re: [Qemu-devel] [PULL v4 07/11] rdma: introduce capability for chunk registration

[Michael S. Tsirkin]

On Sun, Apr 21, 2013 at 12:05:08PM -0400, Michael R. Hines wrote:
> On 04/21/2013 09:19 AM, Paolo Bonzini wrote:
> >Il 20/04/2013 19:02, Michael S. Tsirkin ha scritto:
> >>>>I guess the opposite sense could be named 'x-rdma-pin-all'; default
> >>>>false means to do chunk registration and release,
> >>chunk release only happens after migration is complete unfortunately.
> >>This means that eventually all initialized memory is pinned, regardless
> >>of the setting (this is fixable but there's no plan to fix this, at this
> >>point). So pin-all might be misleading to some.
> >>
> >>I agree 'chunk' is unnecessarily low level though.
> >>The only difference ATM is pinning of uninitialized memory so I think a
> >>better name would be 'x-rdma-pin-uninitialized' or some such.
> >>
> >x-rdma-pin-all is a better choice.  x-rdma-pin-uninitialized is also too
> >low level.
> >
> >Since this series is likely to miss 1.5 at this point, we could
> >implement the unregistration part of the protocol in the destination.
> >This way, any heuristic we add to the source will not break backwards
> >compatibility.
> >
> >Paolo
> >
> 
> The release cycles are relatively fast, according to the website, so
> I don't have any problem with missing 1.5 to make sure that everybody
> is happy and have had a chance to test the software.
>
> But: Let me repeat: we have already discussed in previous emails that
> ibv_reg_mr() => error + notify source + aborted migration would
> be an adequate solution for merging.
>
> Also: let me repeat: We have no intention nor plans (at least not
> from IBM research) to promise to develop nor even explore the
> effects of unregistration in the RDMA protocol as we have zero data
> to show that it does not adversely affect the performance of the solution.
>
> Unless someone puts in the man-hours to show hard data (even with
> micro-benchmark) that migration throughput and migration latency
> performance and migration convergence are not unaffected by such a
> change in the protocol, then such a change would have to be implemented
> as a patch by another member of the community and an option be clearly made
> in the QEMU monitor so that it could be disabled if the user chose to do so.
> 
> - Michael

My interest was in the protocol used, so as long as you don't
intend to enhance the protocol any further, please drop me from the
Cc list on future version of these patches.

I don't take any position on whether your patches should be merged
in their current form.

-- 
MST