[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 3/3] cpus-common: implement dirty limit on vCPU
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 3/3] cpus-common: implement dirty limit on vCPU |
Date: |
Sat, 20 Nov 2021 08:57:36 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
huangy81@chinatelecom.cn writes:
> From: Hyman Huang(黄勇) <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.
>
> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
[...]
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index e948e81..dd65e9e 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -881,6 +881,13 @@ void end_exclusive(void);
> */
> void qemu_init_vcpu(CPUState *cpu);
>
> +/**
> + * dirtylimit_setup:
> + *
> + * dirtylimit setup.
> + */
This is even worse than no documentation.
> +void dirtylimit_setup(int max_cpus);
> +
> #define SSTEP_ENABLE 0x1 /* Enable simulated HW single stepping */
> #define SSTEP_NOIRQ 0x2 /* Do not use IRQ while single stepping */
> #define SSTEP_NOTIMER 0x4 /* Do not Timers while single stepping */
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 358548a..7f6da34 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -527,3 +527,47 @@
> 'data': { '*option': 'str' },
> 'returns': ['CommandLineOptionInfo'],
> 'allow-preconfig': true }
> +
> +##
> +# @DirtyRateQuotaVcpu:
> +#
> +# Dirty rate of vcpu.
> +#
> +# @idx: vcpu index.
> +#
> +# @dirtyrate: dirty rate.
> +#
> +# Since: 6.3
> +#
> +##
> +{ 'struct': 'DirtyRateQuotaVcpu',
> + 'data': { 'idx': 'int', 'dirtyrate': 'uint64' } }
> +
> +##
> +# @set-dirty-limit:
> +#
> +# Since: 6.3
> +#
> +# Example:
> +# {"execute": "set-dirty-limit"}
> +# "arguments": { "idx": "cpu-index",
> +# "dirtyrate": "quota-dirtyrate" } }
The example cannot work: the arguments must be numbers, not strings.
> +#
> +##
> +{ 'command': 'set-dirty-limit',
> + 'data': 'DirtyRateQuotaVcpu' }
Why make DirtyRateQuotaVcpu a separate type? Why not
{ 'command': 'set-dirty-limit',
'data': { 'idx': 'int', 'dirtyrate': 'uint64' } }
> +
> +##
> +# @cancel-dirty-limit:
> +#
> +# @idx: index of vCPU to be canceled
> +#
> +# Since: 6.3
> +#
> +# Example:
> +# {"execute": "cancel-dirty-limit"}
> +# "arguments": { "idx": "cpu-index" } }
> +#
> +##
> +{ 'command': 'cancel-dirty-limit',
> + 'data': { 'idx': 'int' } }
Overall, documentation is too terse. What is a "dirty rate of vcpu",
and why should I care? Is it related to query-dirty-rate?
Nitpick: you use both "vcpu" and "vCPU" in comments. Stick to the
latter, please.
[...]