Re: [PATCH v2 0/6] migration/ram: Optimize for virtio-mem via RamDiscardManager

[Peter Xu]

On Thu, Jul 22, 2021 at 01:43:41PM +0200, David Hildenbrand wrote:
> > > a) In precopy code, always clearing all dirty bits from the bitmap that
> > >     correspond to discarded range, whenever we update the dirty bitmap. 
> > > This
> > >     results in logically unplugged memory to never get migrated.
> > 
> > Have you seen cases where discarded areas are being marked as dirty?
> > That suggests something somewhere is writing to them and shouldn't be.
> 
> I have due to sub-optimal clear_bmap handling to be sorted out by
> 
> 20210722083055.23352-1-wei.w.wang@intel.com">https://lkml.kernel.org/r/20210722083055.23352-1-wei.w.wang@intel.com
> 
> Whereby the issue is rather that initially dirty bits don't get cleared in
> lower layers and keep popping up as dirty.
> 
> The issue with postcopy recovery code setting discarded ranges dirty in
> the dirty bitmap, I did not try reproducing. But from looking at the
> code, it's pretty clear that it would happen.
> 
> Apart from that, nothing should dirty that memory. Of course,
> malicious guests could trigger it for now, in which case we wouldn't catch it
> and migrate such pages with postcopy, because the final bitmap sync in
> ram_postcopy_send_discard_bitmap() is performed without calling notifiers
> right now.

I have the same concern with Dave: does it mean that we don't need to touch at
least ramblock_sync_dirty_bitmap in patch 3?

Doing that for bitmap init and postcopy recovery looks right.

One other trivial comment is instead of touching up ram_dirty_bitmap_reload(),
IMHO it's simpler to set all 1's to disgarded memories on dst receivedmap;
imagine multiple postcopy recovery happened, then with that we walk the disgard
memory list only once for each migration.  Not a big deal, though.

-- 
Peter Xu