[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1] migration: clear the memory region dirty bitmap when skip

From: Peter Xu
Subject: Re: [PATCH v1] migration: clear the memory region dirty bitmap when skipping free pages
Date: Wed, 14 Jul 2021 11:24:44 -0400

On Wed, Jul 14, 2021 at 02:58:31PM +0000, Wang, Wei W wrote:
> On Wednesday, July 14, 2021 6:30 PM, David Hildenbrand wrote:
> > 
> > On 14.07.21 12:27, Michael S. Tsirkin wrote:
> > > On Wed, Jul 14, 2021 at 03:51:04AM -0400, Wei Wang wrote:
> > >> When skipping free pages, their corresponding dirty bits in the
> > >> memory region dirty bitmap need to be cleared. Otherwise the skipped
> > >> pages will be sent in the next round after the migration thread syncs
> > >> dirty bits from the memory region dirty bitmap.
> > >>
> > >> migration_clear_memory_region_dirty_bitmap_range is put outside the
> > >> bitmap_mutex, becasue
> > >
> > > because?
> > >
> > >> memory_region_clear_dirty_bitmap is possible to block on the kvm slot
> > >> mutex (don't want holding bitmap_mutex while blocked on another
> > >> mutex), and clear_bmap_test_and_clear uses atomic operation.
> > 
> > How is that different from our existing caller?
> > 
> > Please either clean everything up, completely avoiding the lock (separate
> > patch), or move it under the lock.
> > 
> > Or am I missing something important?
> That seems ok to me and Peter to have it outside the lock. Not sure if Dave 
> or Juan knows the reason why clear_bmap needs to be under the mutex given 
> that it is atomic operation.

Yes it looks ok to not have the lock to me, but I still think it's easier to
put all bitmap ops under the bitmap_mutex, so we handle clear_bmap/bmap the
same way.  It's also what we did in the existing code (although by accident).

Then we can replace clear_bmap atomic ops to normal mem accesses in a follow up
patch.  But it won't affect a huge lot - unlike normal bmap, clear_bmap is
normally per 1g chunk so modifying clear_bmap happens much less frequently.

Atomic ops will be needed of course if we want a spinlock version of
bitmap_mutex, however I still don't know whether that'll really help anything.


Peter Xu

reply via email to

[Prev in Thread] Current Thread [Next in Thread]