On Thu, Jul 08, 2021 at 02:55:23AM +0000, Wang, Wei W wrote: > On Thursday, July 8, 2021 12:55 AM, Peter Xu wrote: > > On Wed, Jul 07, 2021 at 08:34:50AM +0000, Wang, Wei W wrote: > > > On Wednesday, July 7, 2021 1:47 AM, Peter Xu wrote: > > > > On Sat, Jul 03, 2021 at 02:53:27AM +0000, Wang, Wei W wrote: > > > > > + do { > > > > > + page_to_clear = start + (i++ << > > > > > + block->clear_bmap_shift); > > > > > > > > Why "i" needs to be shifted? > > > > > > Just move to the next clear chunk, no? > > > For example, (1 << 18) pages chunk (i.e. 1GB). > > > > But migration_clear_memory_region_dirty_bitmap() has done the shifting? > > > > Please see this example: start=0, npages = 2 * (1 <<18), i.e. we have 2 > chunks of pages to clear, and start from 0. > First chunk: from 0 to (1 <<18); > Second chunk: from (1 << 18) to 2*(1<<18). > > To clear the second chunk, we need to pass (start + "1 << 18") to > migration_clear_memory_region_dirty_bitmap(), > and clear_bmap_test_and_clear() there will do ">>18" to transform it into the > id of clear_bitmap, which is 1.
I see what you meant, thanks; however I still think it's wrong. + do { + page_to_clear = start + (i++ << block->clear_bmap_shift); + migration_clear_memory_region_dirty_bitmap(ram_state, + block, + page_to_clear); + } while (i <= npages >> block->clear_bmap_shift);. Consider start=1G-1M, npages=2M, this code will only clear log for the 1st 1G chunk, while we need to clear both the 1st and 2nd 1G chunk. If we want to go this route, I think a helper would be great to clear-log for a range of memory too. Thanks, -- Peter Xu