Re: [PATCH v10 3/3] cpus-common: implement dirty page limit on virtual CPU

[Hyman Huang]

在 2021/12/15 16:29, Hyman Huang 写道:
> 在 2021/12/15 16:09, Peter Xu 写道:
> > On Wed, Dec 15, 2021 at 03:56:55PM +0800, Hyman Huang wrote:
> > > > > +{ 'command': 'vcpu-dirty-limit',
> > > > > +  'data': { 'enable': 'bool',
> > > > > +            '*cpu-index': 'uint64',
> > > > > +            '*dirty-rate': 'uint64'} }
> > > >
> > > > Drop @enable, please.
> > > >
> > > > If @dirty-rate is present, set the limit to its value.
> > > >
> > > > If it's absent, cancel the limit.
> > > Ok. Indeed, this is the simplest style. :)
> > >
> > > So the final qmp format should be like:
> > >
> > > case 1: setup vcpu 0 dirty page limit 100MB/s
> > > vcpu-dirty-limit  cpu-index=0   dirty-rate=100MB/s
> > >
> > > case 2: cancle vcpu 0 dirty page limit
> > > vcpu-dirty-limit  cpu-index=0
> >
> > I actually agree with what you said... for human beings no one will 
> > read it as
> > "disable vcpu throttling", instead people could consider it enables vcpu
> > throttle with a default dirty rate from a gut feeling.
> >
> > I think what Markus suggested is the simplest solution for computers, 
> > but it
> > can confuse human beings.  So it turns out to be a general question to 
> > QMP
> > scheme design: should we always assume QMP client to be a piece of 
> > software, or
> > should we still consider the feeling of human beings operating on QMP
> > interfaces using qmp-shell.
> >
> > IMHO we should still consider the latter, if we don't lose much, 
> > anyway.  But I
> > don't have a strong opinion.
> >  > Thanks,
> So, how do you think about it, Markus？

I prefer Peter's advice and there is another reason:

In current implementation, we introduced the global on/off switch,
enable dirty page rate limit on specified vcpu only if @cpu-index is 
passed, otherwise, all vcpu will be affected.

If we remove the @enable and use @dirty-rate to indicate the 
enabled/disable switch. The qmp format should be like the following:

case 1: setup vcpu 0 dirty page limit 100MB/s
vcpu-dirty-limit  cpu-index=0   dirty-rate=100MB/s

case 2: setup all vcpu dirty page limit 100MB/s
vcpu-dirty-limit dirty-rate=100MB/s

case 3: cancel vcpu 0 dirty page limit
vcpu-dirty-limit  cpu-index=0

case 4: cancel all vcpu dirty page limit
vcpu-dirty-limit

For case 4, it seems to be weired.

--
Best regard

Hyman Huang(黄勇)