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

[Peter Xu]

On Tue, Nov 30, 2021 at 06:28:13PM +0800, huangy81@chinatelecom.cn wrote:
>  ##
> +# @set-dirty-limit:
> +#
> +# Set the upper limit of dirty page rate for a virtual CPU.
> +#
> +# Requires KVM with accelerator property "dirty-ring-size" set.
> +# A virtual CPU's dirty page rate is a measure of its memory load.
> +# To observe dirty page rates, use @calc-dirty-rate.
> +#
> +# @cpu-index: index of the virtual CPU.
> +#
> +# @dirty-rate: upper limit for the specified vCPU's dirty page rate (MB/s)
> +#
> +# Since: 7.0
> +#
> +# Example:
> +#   {"execute": "set-dirty-limit"}
> +#    "arguments": { "cpu-index": 0,
> +#                   "dirty-rate": 200 } }
> +#
> +##
> +{ 'command': 'set-dirty-limit',
> +  'data': { 'cpu-index': 'int', 'dirty-rate': 'uint64' } }
> +
> +##
> +# @cancel-dirty-limit:
> +#
> +# Cancel the dirty page limit for the vCPU which has been set with
> +# set-dirty-limit command. Note that this command requires support from
> +# dirty ring, same as the "set-dirty-limit" command.
> +#
> +# @cpu-index: index of the virtual CPU to cancel the dirty page limit
> +#
> +# Since: 7.0
> +#
> +# Example:
> +#   {"execute": "cancel-dirty-limit"}
> +#    "arguments": { "cpu-index": 0 } }
> +#
> +##
> +{ 'command': 'cancel-dirty-limit',
> +  'data': { 'cpu-index': 'int' } }

This seems to be overloaded to be a standalone cmd..

How about:

  { "cmd": "vcpu-dirty-limit",
    "arguments": {
      "cpu": $cpu,
      "enable": true/false,
      "dirty-rate": 100,
    }
  }

If "enable"==false, then "dirty-rate" can be ignored and it'll shut down the
throttling on vcpu N.  Then this command will literally merge the two you
proposed.

It'll be nice if we provide yet another command:

  { "cmd": "query-vcpu-dirty-limit",
    "arguments": {
      "*cpu": $cpu,
    }
  }

When $cpu is specified, we return (cpu=$cpu, real_dirty_rate,
target_dirty_rate) for this vcpu.  When $cpu is not specified, we return an
array of that containing all the vcpus.

It'll be nicer to enhance the output of the query command to e.g. have a global
"enabled"=true/false as long as any vcpu has throttle enabled then the global
throttle is enabled.  I didn't think more than that, but how's that sound so
far?

Thanks,

-- 
Peter Xu