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

[Hyman Huang]

在 2021/11/22 17:10, Markus Armbruster 写道:
> 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.
set-dirty-limit doesn't do any pre-check about if calc-dirty-rate has 
executed, so it doesn't fail.

Since only executing calc-dirty-rate with dirty-ring mode can we get the 
vCPU dirty page rate currently(while the dirty-bitmap only get the vm 
dirty page rate), "One should use ..." maybe misleading, what i actually 
want to say is "One should use the dirty-ring mode to calculate the vCPU 
dirty page rate".
> > > 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.
>
> [...]

--
Best regard

Hyman Huang(黄勇)