Re: [PATCH v10 2/3] cpu-throttle: implement virtual CPU throttle

[Hyman Huang]

在 2021/12/15 15:15, Markus Armbruster 写道:
> huangy81@chinatelecom.cn writes:
>
> > From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> >
> > Setup a negative feedback system when vCPU thread
> > handling KVM_EXIT_DIRTY_RING_FULL exit by introducing
> > throttle_us_per_full field in struct CPUState. Sleep
> > throttle_us_per_full microseconds to throttle vCPU
> > if dirtylimit is enabled.
> >
> > Start a thread to track current dirty page rates and
> > tune the throttle_us_per_full dynamically untill current
> > dirty page rate reach the quota.
> >
> > Introduce the util function in the header for dirtylimit
> > implementation.
> >
> > Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>
> [...]
>
> > diff --git a/qapi/migration.json b/qapi/migration.json
> > index bbfd48c..ac5fa56 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -1850,6 +1850,25 @@
> >   { 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }
> >   
> >   ##
> > +# @DirtyLimitInfo:
> > +#
> > +# Dirty page rate limit information of virtual CPU.
> > +#
> > +# @cpu-index: index of virtual CPU.
> > +#
> > +# @limit-rate: upper limit of dirty page rate for virtual CPU.
>
> If I understand your code correctly, zero means unlimited.  This is
> undocumented.
Ok
> In review of v9, I asked to "make @dirty-rate optional, present means
> enable, absent means disable."  Any particular reason for not doing it
> that way?
No, :(

Actually, i replied the review advice as the following:

-------------
if we drop @enable, enabling dirty limit should be like:
vcpu-dirty-limit cpu-index=0 dirty-rate=1000

And disabling dirty limit like:
vcpu-dirty-limit cpu-index=0

For disabling case, there is no hint of disabling literally in command
"vcpu-dirty-limit".

How about make @dirty-rate optional, when enable dirty limit, it should
present, ignored otherwise?
-------------

The discussion are still ongoing :)

> > +#
> > +# @current-rate: current dirty page rate for virtual CPU.
> > +#
> > +# Since: 7.0
> > +#
> > +##
> > +{ 'struct': 'DirtyLimitInfo',
> > +  'data': { 'cpu-index': 'int',
> > +            'limit-rate': 'int64',
> > +            'current-rate': 'int64' } }
> > +
> > +##
> >   # @snapshot-save:
> >   #
> >   # Save a VM snapshot
>
> [...]

--
Best regard

Hyman Huang(黄勇)