Re: [PATCH v1 07/17] migration/rdma: Use ram_block_discard_set_broken()

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* David Hildenbrand (david@redhat.com) wrote:
> On 15.05.20 19:51, Dr. David Alan Gilbert wrote:
> > * David Hildenbrand (david@redhat.com) wrote:
> >> On 15.05.20 14:45, Dr. David Alan Gilbert wrote:
> >>> * David Hildenbrand (david@redhat.com) wrote:
> >>>> RDMA will pin all guest memory (as documented in docs/rdma.txt). We want
> >>>> to mark RAM block discards to be broken - however, to keep it simple
> >>>> use ram_block_discard_is_required() instead of inhibiting.
> >>>
> >>> Should this be dependent on whether rdma->pin_all is set?
> >>> Even with !pin_all some will be pinned at any given time
> >>> (when it's registered with the rdma stack).
> >>
> >> Do you know how much memory this is? Is such memory only temporarily pinned?
> > 
> > With pin_all not set, only a subset of memory, I think multiple 1MB
> > chunks, are pinned at any one time.
> > 
> >> At least with special-cases of vfio, it's acceptable if some memory is
> >> temporarily pinned - we assume it's only the working set of the driver,
> >> which guests will not inflate as long as they don't want to shoot
> >> themselves in the foot.
> >>
> >> This here sounds like the guest does not know the pinned memory is
> >> special, right?
> > 
> > Right - for RDMA it's all of memory that's being transferred, and the
> > guest doesn't see when each part is transferred.
> 
> 
> Okay, so all memory will eventually be pinned, just not at the same
> time, correct?
> 
> I think this implies that any memory that was previously discarded will
> be backed my new pages, meaning we will consume more memory than intended.
> 
> If so, always disabling discarding of RAM seems to be the right thing to do.

Yeh that's probably true, although there's a check for 'buffer_is_zero'
in the !rdma->pin_all case, if the entire area is zero (or probably if
unmapped) then it sends a notification rather than registering; see
qemu_rdma_write_one and search for 'This chunk has not yet been
registered, so first check to see'

Dave

> 
> -- 
> Thanks,
> 
> David / dhildenb
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK