[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: |
Fri, 9 Feb 2018 09:13:28 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
On 08.02.2018 22:41, Eduardo Habkost wrote:
> On Thu, Feb 08, 2018 at 02:59:17PM -0600, Eric Blake wrote:
>> On 02/08/2018 01:59 PM, Eduardo Habkost wrote:
>>> On Wed, Feb 07, 2018 at 12:50:13PM -0500, 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:
>>>>
>>
>>>> +# Notes: @halted is a transient state that changes frequently. By the
>>>> time the
>>>> +# data is sent to the client, the guest may no longer be halted.
>>>> +##
>>>> +{ 'struct': 'CpuInfo2',
>>>> + 'data': {'cpu-index': 'int', '*halted': 'bool', 'qom-path': 'str',
>>>> + 'thread-id': 'int', '*props': 'CpuInstanceProperties' } }
>>>
>>> This will require duplicating struct fields every time we add a
>>> new field to query-cpus-fast (e.g. how would VIktor's
>>> CpuInfoS390State patch[1] look like if rebased on top of yours?).
>>>
>>> One way to avoid that is to use CpuInfo for both, and make all
>>> "slow" fields optional. Another option is to use QAPI
>>> inheritance, but it could be a little complicated if unions are
>>> involved?
>>
>> Inheritance is better than optional fields for the sake of introspection
>> learning which fields to expect.
>>
>> Put the common fields to both interfaces in the base class, then have the
>> slower (older) CpuInfo class extend the base class to add the additional
>> fields.
>>
>> Unions should be able to inherit just fine from structs (after all, a flat
>> union requires a struct base); but if we need two layers of unions, we'll
>> need to enhance QAPI code generation first.
>
> If we can't do union-union inheritance yet, maybe we can work around it this
> way:
>
> # fields that are always returned by both query-cpus and query-cpus-fast
> { 'struct': 'BothCpuInfoBase',
> 'data': {'cpu': 'int', 'qom_path': 'str', 'thread_id': 'int',
> '*props': 'CpuInstanceProperties' } }
>
> # fields that are always returned by query-cpus
> { 'struct': 'CpuInfoBase',
> 'base': 'BothCpuInfoBase',
> 'data': {'current': 'bool', 'halted': 'bool', 'arch': 'CpuInfoArch' } }
>
> # query-cpus return value
> { 'union': 'CpuInfo',
> 'base': 'CpuInfoBase',
> 'discriminator': 'arch',
> 'data': { 'x86': 'CpuInfoX86',
> 's390': 'CpuInfoS390',
> 'sparc': 'CpuInfoSPARC',
> 'ppc': 'CpuInfoPPC',
> 'mips': 'CpuInfoMIPS',
> 'tricore': 'CpuInfoTricore',
> 'other': 'CpuInfoOther' } }
>
> # fields that are always returned by query-cpus-fast
> { 'struct': 'FastCpuInfoBase',
> 'base': 'BothCpuInfoBase',
> 'data': { 'arch': 'CpuInfoArch' } }
>
> # return value of query-cpus-fast
> { 'union': 'FastCpuInfo',
> 'base': 'FastCpuInfoBase',
> 'discriminator': 'arch',
> 'data': { 'x86': 'CpuInfoOther',
> 's390': 'FastCpuInfoS390',
> 'sparc': 'CpuInfoOther',
> 'ppc': 'CpuInfoOther',
> 'mips': 'CpuInfoOther',
> 'tricore': 'CpuInfoOther',
> 'other': 'CpuInfoOther' } }
>
> # fields returned by both query-cpus and query-cpus-fast on s390
> { 'struct': 'FastCpuInfoS390',
> 'data': { 'fast_field': 'int' } }
>
> # fields returned by query-cpus on s390
> { 'struct': 'CpuInfoS390',
> 'base': 'FastCpuInfoS390',
> 'data': { 'slow_field': 'int' } }
>
This is not exactly pretty, but would provide the functionality (and
flexibility) I'd expect.
--
Regards,
Viktor Mihajlovski
- Re: [Qemu-devel] [PATCH 2/2] qmp: document query-cpus performance issue, (continued)