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

[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.

>  
>  ##
>  # @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.

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"?

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?

-- 
Peter Xu