Re: [PATCH RFC 04/15] KVM: Implement ring-based dirty memory tracking

[Peter Xu]

On Tue, Dec 17, 2019 at 01:19:05PM +0100, Paolo Bonzini wrote:
> On 17/12/19 13:16, Christophe de Dinechin wrote:
> > 
> > 
> >> On 14 Dec 2019, at 08:57, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> >>
> >> On 13/12/19 21:23, Peter Xu wrote:
> >>>> What is the benefit of using u16 for that? That means with 4K pages, you
> >>>> can share at most 256M of dirty memory each time? That seems low to me,
> >>>> especially since it's sufficient to touch one byte in a page to dirty it.
> >>>>
> >>>> Actually, this is not consistent with the definition in the code ;-)
> >>>> So I'll assume it's actually u32.
> >>> Yes it's u32 now.  Actually I believe at least Paolo would prefer u16
> >>> more. :)
> >>
> >> It has to be u16, because it overlaps the padding of the first entry.
> > 
> > Wow, now thatâs subtle.
> > 
> > That definitely needs a union with the padding to make this explicit.
> > 
> > (My guess is you do that to page-align the whole thing and avoid adding a
> > page just for the counters)

(Just to make sure this is clear... Paolo was talking about the
 previous version.  This version does not have this limitation because
 we don't have that union definition any more)

> 
> Yes, that was the idea but Peter decided to scrap it. :)

There's still time to persuade me to going back to it. :)

(Though, yes I still like current solution... if we can get rid of the
 only kvmgt ugliness, we can even throw away the per-vm ring with its
 "extra" 4k page.  Then I suppose it'll be even harder to persuade me :)

-- 
Peter Xu