Re: [PATCH v3 3/3] cpus-common: implement dirty limit on vCPU

[Hyman Huang <huangy81@chinatelecom.cn>]

在 2021/11/22 15:35, Markus Armbruster 写道:
> huangy81@chinatelecom.cn writes:
> 
>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>
>> implement dirtyrate calculation periodically basing on
>> dirty-ring and throttle vCPU until it reachs the quota
>> dirtyrate given by user.
>>
>> introduce qmp commands set-dirty-limit/cancel-dirty-limit to
>> set/cancel dirty limit on vCPU.
> 
> Please start sentences with a capital letter.
> 
Ok，i'll check the syntax problem next version.
>>
>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> 
> [...]
> 
>> diff --git a/qapi/misc.json b/qapi/misc.json
>> index 358548a..98e6001 100644
>> --- a/qapi/misc.json
>> +++ b/qapi/misc.json
>> @@ -527,3 +527,42 @@
>>    'data': { '*option': 'str' },
>>    'returns': ['CommandLineOptionInfo'],
>>    'allow-preconfig': true }
>> +
>> +##
>> +# @set-dirty-limit:
>> +#
>> +# This command could be used to cap the vCPU memory load, which is also
>> +# refered as dirtyrate. One should use "calc-dirty-rate" with "dirty-ring"
>> +# and to calculate vCPU dirtyrate and query it with "query-dirty-rate".
>> +# Once getting the vCPU current dirtyrate, "set-dirty-limit" can be used
>> +# to set the upper limit of dirtyrate for the interested vCPU.
> 
> "dirtyrate" is not a word.  Let's spell it "dirty page rate", for
> consistency with the documentation in migration.json.
Ok, sounds good.
> 
> Regarding "One should use ...": sounds like you have to run
> calc-dirty-rate with argument @mode set to @dirty-ring before this
> command.  Correct?  What happens when you don't?  set-dirty-limit fails?
> 
> Do you also have to run query-dirty-rate before this command?
Actually no, i'll clarify the usage next verison.
> 
> Speaking of migration.json: should these commands be defined there, next
> to calc-dirty-rate and query-dirty-rate?
I'm struggling too because these commands will be used in migration but 
it is vCPU they handle.
> 
>> +#
>> +# @idx: vCPU index to set dirtylimit.
>> +#
>> +# @dirtyrate: upper limit of drityrate the specified vCPU could reach (MB/s)
> 
> Typo "drityrate".
> 
> Suggest "upper limit for the specified vCPU's dirty page rate (MB/s)".
Ok.
> 
>> +#
>> +# Since: 6.3
>> +#
>> +# Example:
>> +#   {"execute": "set-dirty-limit"}
>> +#    "arguments": { "idx": 0,
>> +#                   "dirtyrate": 200 } }
>> +#
>> +##
>> +{ 'command': 'set-dirty-limit',
>> +  'data': { 'idx': 'int', 'dirtyrate': 'uint64' } }
>> +
>> +##
>> +# @cancel-dirty-limit:
>> +#
>> +# @idx: vCPU index to canceled the dirtylimit
>> +#
>> +# Since: 6.3
>> +#
>> +# Example:
>> +#   {"execute": "cancel-dirty-limit"}
>> +#    "arguments": { "idx": 0 } }
>> +#
>> +##
>> +{ 'command': 'cancel-dirty-limit',
>> +  'data': { 'idx': 'int' } }
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index 1159a64..170ee23 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -3776,5 +3776,6 @@ void qemu_init(int argc, char **argv, char **envp)
>>       qemu_init_displays();
>>       accel_setup_post(current_machine);
>>       os_setup_post();
>> +    dirtylimit_setup(current_machine->smp.max_cpus);
>>       resume_mux_open();
>>   }
> 

-- 
Best regard

Hyman Huang(黄勇)