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

[Markus Armbruster <armbru@redhat.com>]

Hyman Huang <huangy81@chinatelecom.cn> writes:

> =E5=9C=A8 2021/11/22 15:35, Markus Armbruster =E5=86=99=E9=81=93:
>> huangy81@chinatelecom.cn writes:
>> 
>>> From: Hyman Huang(=E9=BB=84=E5=8B=87) <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?

You didn't answer this question.

>> Do you also have to run query-dirty-rate before this command?
> Actually no, i'll clarify the usage next verison.

Regarding "dirty-ring": is this merely a limitation of the
implementation, i.e. other modes could be made to work if we cared, or
is it more fundamental?

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

I think migration.json is more about CPUs than misc.json is.  Let's add
the new commands to migration.json if migration maintainers are okay
with it.

[...]