Re: [PATCH v2 08/11] migration: Export dirty-limit time info

[Markus Armbruster <armbru@redhat.com>]

Hyman <huangy81@chinatelecom.cn> writes:

> 在 2022/11/22 0:26, huangy81@chinatelecom.cn 写道:
>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>> Export dirty limit throttle time and estimated ring full
>> time, through which we can observe the process of dirty
>> limit during live migration.
>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>> ---
>>   include/sysemu/dirtylimit.h |  2 ++
>>   migration/migration.c       | 10 ++++++++++
>>   monitor/hmp-cmds.c          | 10 ++++++++++
>>   qapi/migration.json         | 10 +++++++++-
>>   softmmu/dirtylimit.c        | 31 +++++++++++++++++++++++++++++++
>>   5 files changed, 62 insertions(+), 1 deletion(-)

[...]

>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index af6b2da..62db5cb 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -242,6 +242,12 @@
>>  #                   Present and non-empty when migration is blocked.
>>  #                   (since 6.0)
>>  #
>> +# @dirty-limit-throttle-us-per-full: Throttle time (us) during the period of
>> +#                                    dirty ring full (since 7.1)
>> +#
>> +# @dirty-limit-us-ring-full: Estimated periodic time (us) of dirty ring full.
>> +#                            (since 7.1)
> How about the following documents:
>
> # @dirty-limit-throttletime-each-round: Max throttle time (us) of all virtual CPUs each dirty ring
> #                                       full round, used to observe if dirty-limit take effect
> #                                       during live migration. (since 7.3)
> #
> # @dirty-limit-ring-full-time: Estimated average dirty ring full time (us) each round, note that
> #                              the value equals dirty ring memory size divided by average dirty
> #                              page rate of virtual CPU, which can be used to observe the average
> #                              memory load of virtual CPU indirectly. (since 7.3)
>
> Is it more easy-understanding ?

dirty-limit-ring-full-time is better than dirty-limit-us-ring-full.

dirty-limit-throttletime-each-round is rather long.

We say "in microseconds" in doc comments.

Avoid abbreviations like "max" in doc comments, spell them out like
"maximum".

I need to give the text a closer read.  Out of time for today.  If you
don't see a reply from me early next week, feel free to remind me.

>> +#
>>  # Since: 0.14
>>  ##
>>  { 'struct': 'MigrationInfo',
>> @@ -259,7 +265,9 @@
>>             '*postcopy-blocktime' : 'uint32',
>>             '*postcopy-vcpu-blocktime': ['uint32'],
>>             '*compression': 'CompressionStats',
>> -           '*socket-address': ['SocketAddress'] } }
>> +           '*socket-address': ['SocketAddress'],
>> +           '*dirty-limit-throttle-us-per-full': 'int64',
>> +           '*dirty-limit-us-ring-full': 'int64'} }
>>  ##
>>  # @query-migrate:

[...]