On Fri, Dec 11, 2020 at 09:13:10AM +0800, zhukeqian wrote: > > On 2020/12/10 22:50, Peter Xu wrote: > > On Thu, Dec 10, 2020 at 10:53:23AM +0800, zhukeqian wrote: > >> > >> > >> On 2020/12/10 10:08, Peter Xu wrote: > >>> Keqian, > >>> > >>> On Thu, Dec 10, 2020 at 09:46:06AM +0800, zhukeqian wrote: > >>>> Hi, > >>>> > >>>> I see that if start or size is not PAGE aligned, it also clears areas > >>>> which beyond caller's expectation, so do we also need to consider this? > >>> > >>> Could you elaborate? > >>> > >>> If start_delta != 0, kvm_log_clear_one_slot() should already go the slow > >>> path. > >>> > >>> Thanks, > >>> > >> > >> Hi Peter, > >> > >> start_delta /= psize; > >> > >> If start is not PAGE aligned, then start_delta is not PAGE aligned. > >> so I think the above code will implicitly extend our start to be PAGE > >> aligned. > >> > >> I suggest that we should shrink the start and (start + size) to be PAGE > >> aligned > >> at beginning of this function. > > > > Callers should be with TARGET_PAGE_SIZE aligned on the size, so at least > > x86_64 > > should be pretty safe since host/guest page sizes match. > > > > Though indeed I must confess I don't know how it worked in general when host > > page size != target page size, at least for migration. For example, I > > believe > > kvm dirty logging is host page size based, though migration should be > > migrating > > pages in guest page size granule when it spots a dirty bit set. > > > Hi, > > Indeed, we handle target_page_size aligned @start and @size in general. Maybe > we'd better > add explicit function comments about alignment requirement, and explicit > alignment assert > on @start and @size?
Yes we can, but I think it's not strongly necessary. As Zenghui pointed out, the callers of memory_region_clear_dirty_bitmap() should always be aware of the fact that dirty bitmap is always page size based. OTOH I'm more worried on the other question on how we handle guest psize != host psize case for migration now... Thanks, -- Peter Xu
