[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] qmp: add query-cpus-fast
From: |
Viktor Mihajlovski |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] qmp: add query-cpus-fast |
Date: |
Thu, 8 Feb 2018 08:41:31 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
On 07.02.2018 18:50, Luiz Capitulino wrote:
> The query-cpus command has an extremely serious side effect:
> it always interrupt all running vCPUs so that they can run
> ioctl calls. This can cause a huge performance degradation for
> some workloads. And most of the information retrieved by the
> ioctl calls are not even used by query-cpus.
>
> This commit introduces a replacement for query-cpus called
> query-cpus-fast, which has the following features:
>
> o Never interrupt vCPUs threads. query-cpus-fast only returns
> vCPU information maintained by QEMU itself, which should be
> sufficient for most management software needs
>
> o Make "halted" field optional: we only return it if the
> halted state is maintained by QEMU. But this also gives
> the option of dropping the field in the future (see below)
>
> o Drop irrelevant fields such as "current", "pc" and "arch"
I disagree that arch is irrelevant and would strongly suggest to keep
arch and arch-specific fields. At least in the case of s390 there's a
cpu_state field that can be obtained cheaply.
[...]
> diff --git a/cpus.c b/cpus.c
> index 2cb0af9b22..3b68a8146c 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -2083,6 +2083,50 @@ CpuInfoList *qmp_query_cpus(Error **errp)
> return head;
> }
>
> +/*
> + * fast means: we NEVER interrupt vCPU threads to retrieve
> + * information from KVM.
> + */
> +CpuInfo2List *qmp_query_cpus_fast(Error **errp)
> +{
> + MachineState *ms = MACHINE(qdev_get_machine());
> + MachineClass *mc = MACHINE_GET_CLASS(ms);
> + CpuInfo2List *head = NULL, *cur_item = NULL;
> + CPUState *cpu;
> +
> + CPU_FOREACH(cpu) {
> + CpuInfo2List *info = g_malloc0(sizeof(*info));
> + info->value = g_malloc0(sizeof(*info->value));
> +
> + info->value->cpu_index = cpu->cpu_index;
> + info->value->qom_path = object_get_canonical_path(OBJECT(cpu));
> + info->value->thread_id = cpu->thread_id;
> +
> + info->value->has_props = !!mc->cpu_index_to_instance_props;
> + if (info->value->has_props) {
> + CpuInstanceProperties *props;
> + props = g_malloc0(sizeof(*props));
> + *props = mc->cpu_index_to_instance_props(ms, cpu->cpu_index);
> + info->value->props = props;
> + }
> +
> + /* if in kernel irqchip is used, we don't have 'halted' */
> + info->value->has_halted = !kvm_irqchip_in_kernel();
This is definitely not true for s390. Externally observable CPU state
changes are handled by QEMU there. We may still drop halted if we add a
more appropriate arch-specific field.
> + if (info->value->has_halted) {
> + info->value->halted = cpu->halted;
> + }
[...]
--
Regards,
Viktor Mihajlovski