Re: [RFC PATCH] iothread: add set_iothread_poll_* commands

[Zhenyu Ye <yezhenyu2@huawei.com>]

On 2019/10/23 4:40, Eric Blake wrote:
> On 10/22/19 3:12 AM, yezhenyu (A) wrote:
>> Since qemu2.9, QEMU added three AioContext poll parameters to struct
>> IOThread: poll_max_ns, poll_grow and poll_shrink. These properties are
>> used to control iothread polling time.
>>
>> However, there isn't properly hmp commands to adjust them when the VM is
>> alive. It's useful to adjust them online when observing the impact of
>> different property value on performance.
>>
>> This patch add three hmp commands to adjust iothread poll-* properties
>> for special iothread:
>>
>> set_iothread_poll_max_ns: set the maximum polling time in ns;
>> set_iothread_poll_grow: set how many ns will be added to polling time;
>> set_iothread_poll_shrink: set how many ns will be removed from polling
>> time.
>>
>> Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com>
>> ---
>> hmp-commands.hx | 42 ++++++++++++++++++++
>> hmp.c | 30 +++++++++++++++
>> hmp.h | 3 ++
>> include/sysemu/iothread.h | 6 +++
>> iothread.c | 80 +++++++++++++++++++++++++++++++++++++++
>> qapi/misc.json | 23 +++++++++++
>> 6 files changed, 184 insertions(+)
> 
> Looking at just the QMP...
> 

Thanks for your review. I will split this patch to QMP and HMP.

>> +++ b/qapi/misc.json
>> @@ -675,6 +675,29 @@
>> { 'command': 'query-iothreads', 'returns': ['IOThreadInfo'],
>> 'allow-preconfig': true }
>>
>> +##
>> +# @set-iothread-poll-param:
>> +#
>> +# Set poll-* properties for a special iothread.
>> +# The poll-* name can be poll_max_ns/poll_grow/poll_shrink.
> 
> This should be an enum.

I will change the name argument to ENUM, such as,

    { 'enum': 'IothreadPollProperty',
      'data': [ 'max-ns', 'grow', 'shrink' ] }

>> +#
>> +# Notes: can not set the QEMU main loop thread, which is not declared
>> +# using the -object iothread command-line option. The poll_ns property can not
>> +# be set manually, which is auto-adjust.
> 
> You failed to document the parameters (iothread_id, name, value).
> 

I will add these documents.

>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "set-iothread-poll-param",
>> +# "arguments": { "iothread_id": "1",
>> +# "name": "poll_max_ns",
>> +# "value": 1000 } }
>> +# <- { "return": {} }
>> +#
>> +# Since 3.0
> 
> 4.2 is the earliest this can make it in.
> 

OK, I will change this to 4.2.

>> +##
>> +{ 'command': 'set-iothread-poll-param',
>> + 'data': {'iothread_id': 'str', 'name': 'str', 'value': 'int'} }
> 
> Our naming convention prefers 'iothread-id'.
> 

ok, I will correct it, such as,

    { 'command': 'set-iothread-poll-param',
    'data': {'iothread-id': 'str', 'name': 'IothreadPollProperty', 'value': 'int'} }