Re: [PATCH v3 4/7] migration/dirtyrate: add per-vcpu option for calc-dirty-rate

[Hyman Huang]

在 2021/6/7 23:46, Peter Xu 写道:
> On Mon, Jun 07, 2021 at 09:12:36AM +0800, huangy81@chinatelecom.cn wrote:
> > @@ -1751,7 +1770,9 @@
> >              'status': 'DirtyRateStatus',
> >              'start-time': 'int64',
> >              'calc-time': 'int64',
> > -           'sample-pages': 'uint64'} }
> > +           '*sample-pages': 'uint64',
> > +           'per-vcpu': 'bool',
> > +           '*vcpu-dirty-rate': [ 'DirtyRateVcpu' ] } }
>
> Ideally we shouldn't touch existing exported fields?  I know it's not a problem
> since it's just introduced and together with the series..  The other
> alternative is to keep it as is but just ignore it in the result (or set it to
> zero for per-vcpu sampling)?  No strong opinion.
i'll change 'sample-pages' to '*sample-pages' in the first commit 
"migration/dirtyrate: make sample page count configurable" so that the 
following patch won't touch existing exported field.
> >   
> >   ##
> >   # @calc-dirty-rate:
> > @@ -1760,6 +1781,10 @@
> >   #
> >   # @calc-time: time in units of second for sample dirty pages
> >   #
> > +# @per-vcpu: calculate vcpu dirty rate if true, the default value is
> > +#            false, note that the per-vcpu and sample-pages are mutually
> > +#            exclusive (since 6.1)
> > +#
> >   # @sample-pages: page count per GB for sample dirty pages
> >   #                the default value is 512 (since 6.1)
> >   #
> > @@ -1769,7 +1794,7 @@
> >   #   {"command": "calc-dirty-rate", "data": {"calc-time": 1, 'sample-pages': 
> > 512} }
> >   #
> >   ##
> > -{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64', 
> > '*sample-pages': 'int'} }
> > +{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64', 'per-vcpu': 
> > 'bool', '*sample-pages': 'int'} }
>
> I still think exporting this new feature should happen as/in the last patch.
indeed, the last version ignore this advice, i'll introduce the 
"per-vcpu" in the last patch, before it implemented.
> Also, wondering whether the name "per-vcpu" would let people think of "samping
> pages per-vcpu only", not showing that it's a completely new mechanism inside.
> How about make it a new parameter "*mode" (and I think even with per-vcpu it
> should be optional as "*per-vcpu") default to "page-sampling" and add a new
> "dirty-ring"?
i prefer the "*mode" parameter to show the different dirty rate method.
> Actually I'm also wondering whether dirty log could be anything useful here in
> the future as a 3rd mode (then the "*mode" idea should be more useful if so),
> basically for old kernels where dirty ring is not there, we can timely call
> memory_global_dirty_log_sync() and calculate dirty pages there just like what
> we do with dirty rings (without calling migration_bitmap_sync(), so we don't
> need to deliver dirty bits from kvmslots to ramblocks, just pick them up from
> kvm and do the accounting for pure dirty rate measurement).  That's a wild idea
> though, so just raise it up.  Would that be anything useful?
uh, actually this idea is what i'm working on to stat the memory 
heat(trying to reduce the transferred memory in migration), 
theoretically it can be used for dirty rate measurement also, maybe i 
could try it after this patchset work having done.
>

--
Best regard

Hyman Huang(黄勇)