[PATCH RFC v1 3/3] qapi/iothread: introduce poll-weight parameter for aio-poll

Jaehoon Kim posted 3 patches 3 weeks, 5 days ago
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>, "Dr. David Alan Gilbert" <dave@treblig.org>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Stefan Weil <sw@weilnetz.de>
[PATCH RFC v1 3/3] qapi/iothread: introduce poll-weight parameter for aio-poll
Posted by Jaehoon Kim 3 weeks, 5 days ago
Introduce a new poll-weight parameter for aio-poll. This parameter
controls how much the most recent event interval affects the next
polling duration. When set to 0, a default value of 2 is used, meaning
the current interval contributes roughly 25% to the calculation. Larger
values decrease the weight of the current interval, enabling more
gradual adjustments to polling duration.

Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
---
 include/qemu/aio.h                |  4 +++-
 include/system/iothread.h         |  1 +
 iothread.c                        | 10 ++++++++++
 monitor/hmp-cmds.c                |  1 +
 qapi/misc.json                    |  6 ++++++
 qapi/qom.json                     |  8 +++++++-
 qemu-options.hx                   |  7 ++++++-
 tests/unit/test-nested-aio-poll.c |  2 +-
 util/aio-posix.c                  |  9 ++++++---
 util/aio-win32.c                  |  3 ++-
 util/async.c                      |  1 +
 11 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/include/qemu/aio.h b/include/qemu/aio.h
index 6c77a190e9..50b8db2712 100644
--- a/include/qemu/aio.h
+++ b/include/qemu/aio.h
@@ -311,6 +311,7 @@ struct AioContext {
     int64_t poll_max_ns;    /* maximum polling time in nanoseconds */
     int64_t poll_grow;      /* polling time growth factor */
     int64_t poll_shrink;    /* polling time shrink factor */
+    int64_t poll_weight;    /* weight of current interval in calculation */
 
     /* AIO engine parameters */
     int64_t aio_max_batch;  /* maximum number of requests in a batch */
@@ -792,12 +793,13 @@ void aio_context_destroy(AioContext *ctx);
  * @max_ns: how long to busy poll for, in nanoseconds
  * @grow: polling time growth factor
  * @shrink: polling time shrink factor
+ * @weight: weight factor applied to the current polling interval
  *
  * Poll mode can be disabled by setting poll_max_ns to 0.
  */
 void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns,
                                  int64_t grow, int64_t shrink,
-                                 Error **errp);
+                                 int64_t weight, Error **errp);
 
 /**
  * aio_context_set_aio_params:
diff --git a/include/system/iothread.h b/include/system/iothread.h
index e26d13c6c7..6ea57ed126 100644
--- a/include/system/iothread.h
+++ b/include/system/iothread.h
@@ -38,6 +38,7 @@ struct IOThread {
     int64_t poll_max_ns;
     int64_t poll_grow;
     int64_t poll_shrink;
+    int64_t poll_weight;
 };
 typedef struct IOThread IOThread;
 
diff --git a/iothread.c b/iothread.c
index caf68e0764..68a944e57c 100644
--- a/iothread.c
+++ b/iothread.c
@@ -164,6 +164,7 @@ static void iothread_set_aio_context_params(EventLoopBase *base, Error **errp)
                                 iothread->poll_max_ns,
                                 iothread->poll_grow,
                                 iothread->poll_shrink,
+                                iothread->poll_weight,
                                 errp);
     if (*errp) {
         return;
@@ -233,6 +234,9 @@ static IOThreadParamInfo poll_grow_info = {
 static IOThreadParamInfo poll_shrink_info = {
     "poll-shrink", offsetof(IOThread, poll_shrink),
 };
+static IOThreadParamInfo poll_weight_info = {
+    "poll-weight", offsetof(IOThread, poll_weight),
+};
 
 static void iothread_get_param(Object *obj, Visitor *v,
         const char *name, IOThreadParamInfo *info, Error **errp)
@@ -288,6 +292,7 @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
                                     iothread->poll_max_ns,
                                     iothread->poll_grow,
                                     iothread->poll_shrink,
+                                    iothread->poll_weight,
                                     errp);
     }
 }
@@ -311,6 +316,10 @@ static void iothread_class_init(ObjectClass *klass, const void *class_data)
                               iothread_get_poll_param,
                               iothread_set_poll_param,
                               NULL, &poll_shrink_info);
+    object_class_property_add(klass, "poll-weight", "int",
+                              iothread_get_poll_param,
+                              iothread_set_poll_param,
+                              NULL, &poll_weight_info);
 }
 
 static const TypeInfo iothread_info = {
@@ -356,6 +365,7 @@ static int query_one_iothread(Object *object, void *opaque)
     info->poll_max_ns = iothread->poll_max_ns;
     info->poll_grow = iothread->poll_grow;
     info->poll_shrink = iothread->poll_shrink;
+    info->poll_weight = iothread->poll_weight;
     info->aio_max_batch = iothread->parent_obj.aio_max_batch;
 
     QAPI_LIST_APPEND(*tail, info);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 5a673cddb2..40e3b1da50 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -205,6 +205,7 @@ void hmp_info_iothreads(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "  poll-max-ns=%" PRId64 "\n", value->poll_max_ns);
         monitor_printf(mon, "  poll-grow=%" PRId64 "\n", value->poll_grow);
         monitor_printf(mon, "  poll-shrink=%" PRId64 "\n", value->poll_shrink);
+        monitor_printf(mon, "  poll-weight=%" PRId64 "\n", value->poll_weight);
         monitor_printf(mon, "  aio-max-batch=%" PRId64 "\n",
                        value->aio_max_batch);
     }
diff --git a/qapi/misc.json b/qapi/misc.json
index 28c641fe2f..b21cc48a03 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -85,6 +85,11 @@
 # @poll-shrink: how many ns will be removed from polling time, 0 means
 #     that it's not configured (since 2.9)
 #
+# @poll-weight: the weight factor for adaptive polling.
+#     Determines how much the current event interval contributes to
+#     the next polling time calculation.  0 means that the default
+#     value is used.  (since 10.1)
+#
 # @aio-max-batch: maximum number of requests in a batch for the AIO
 #     engine, 0 means that the engine will use its default (since 6.1)
 #
@@ -96,6 +101,7 @@
            'poll-max-ns': 'int',
            'poll-grow': 'int',
            'poll-shrink': 'int',
+           'poll-weight': 'int',
            'aio-max-batch': 'int' } }
 
 ##
diff --git a/qapi/qom.json b/qapi/qom.json
index 6f5c9de0f0..d90823478d 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -606,6 +606,11 @@
 #     algorithm detects it is spending too long polling without
 #     encountering events.  0 selects a default behaviour (default: 0)
 #
+# @poll-weight: the weight factor for adaptive polling.
+#     Determines how much the current event interval contributes to
+#     the next polling time calculation.  0 selects a default
+#     behaviour (default: 0) since 10.1.
+#
 # The @aio-max-batch option is available since 6.1.
 #
 # Since: 2.0
@@ -614,7 +619,8 @@
   'base': 'EventLoopBaseProperties',
   'data': { '*poll-max-ns': 'int',
             '*poll-grow': 'int',
-            '*poll-shrink': 'int' } }
+            '*poll-shrink': 'int',
+            '*poll-weight': 'int' } }
 
 ##
 # @MainLoopProperties:
diff --git a/qemu-options.hx b/qemu-options.hx
index ec92723f10..74adaf55fc 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -6352,7 +6352,7 @@ SRST
 
             CN=laptop.example.com,O=Example Home,L=London,ST=London,C=GB
 
-    ``-object iothread,id=id,poll-max-ns=poll-max-ns,poll-grow=poll-grow,poll-shrink=poll-shrink,aio-max-batch=aio-max-batch``
+    ``-object iothread,id=id,poll-max-ns=poll-max-ns,poll-grow=poll-grow,poll-shrink=poll-shrink,poll-weight=poll-weight,aio-max-batch=aio-max-batch``
         Creates a dedicated event loop thread that devices can be
         assigned to. This is known as an IOThread. By default device
         emulation happens in vCPU threads or the main event loop thread.
@@ -6388,6 +6388,11 @@ SRST
         the polling time when the algorithm detects it is spending too
         long polling without encountering events.
 
+        The ``poll-weight`` parameter is the weight factor used in the
+        adaptive polling algorithm. It determines how much the most
+        recent event interval affects the calculation of the next
+        polling duration.
+
         The ``aio-max-batch`` parameter is the maximum number of requests
         in a batch for the AIO engine, 0 means that the engine will use
         its default.
diff --git a/tests/unit/test-nested-aio-poll.c b/tests/unit/test-nested-aio-poll.c
index 9ab1ad08a7..ed791fa23b 100644
--- a/tests/unit/test-nested-aio-poll.c
+++ b/tests/unit/test-nested-aio-poll.c
@@ -81,7 +81,7 @@ static void test(void)
     qemu_set_current_aio_context(td.ctx);
 
     /* Enable polling */
-    aio_context_set_poll_params(td.ctx, 1000000, 2, 2, &error_abort);
+    aio_context_set_poll_params(td.ctx, 1000000, 2, 2, 0, &error_abort);
 
     /* Make the event notifier active (set) right away */
     event_notifier_init(&td.poll_notifier, 1);
diff --git a/util/aio-posix.c b/util/aio-posix.c
index dd6008898b..d4f3d8ca8f 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -630,6 +630,7 @@ static void adjust_block_ns(AioContext *ctx, int64_t block_ns)
 {
     AioHandler *node;
     int64_t adj_block_ns = -1;
+    int64_t poll_weight = ctx->poll_weight ? : POLL_WEIGHT_SHIFT;
 
     QLIST_FOREACH(node, &ctx->poll_aio_handlers, node_poll) {
         if (node->poll.has_event) {
@@ -639,8 +640,8 @@ static void adjust_block_ns(AioContext *ctx, int64_t block_ns)
              * poll.ns to smooth out polling time adjustments.
              */
             node->poll.ns = node->poll.ns
-                ? (node->poll.ns - (node->poll.ns >> POLL_WEIGHT_SHIFT))
-                + (block_ns >> POLL_WEIGHT_SHIFT) : block_ns;
+                ? (node->poll.ns - (node->poll.ns >> poll_weight))
+                + (block_ns >> poll_weight) : block_ns;
 
             if (node->poll.ns >= ctx->poll_max_ns) {
                 node->poll.ns = 0;
@@ -830,7 +831,8 @@ void aio_context_destroy(AioContext *ctx)
 }
 
 void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns,
-                                 int64_t grow, int64_t shrink, Error **errp)
+                                 int64_t grow, int64_t shrink,
+                                 int64_t weight, Error **errp)
 {
     AioHandler *node;
 
@@ -847,6 +849,7 @@ void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns,
     ctx->poll_max_ns = max_ns;
     ctx->poll_grow = grow;
     ctx->poll_shrink = shrink;
+    ctx->poll_weight = weight;
     ctx->poll_ns = 0;
 
     aio_notify(ctx);
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 6e6f699e4b..1985843233 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -429,7 +429,8 @@ void aio_context_destroy(AioContext *ctx)
 }
 
 void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns,
-                                 int64_t grow, int64_t shrink, Error **errp)
+                                 int64_t grow, int64_t shrink,
+                                 int64_t weight, Error **errp)
 {
     if (max_ns) {
         error_setg(errp, "AioContext polling is not implemented on Windows");
diff --git a/util/async.c b/util/async.c
index 9d3627566f..741fcfd6a7 100644
--- a/util/async.c
+++ b/util/async.c
@@ -609,6 +609,7 @@ AioContext *aio_context_new(Error **errp)
     ctx->poll_ns = 0;
     ctx->poll_grow = 0;
     ctx->poll_shrink = 0;
+    ctx->poll_weight = 0;
 
     ctx->aio_max_batch = 0;
 
-- 
2.50.1
Re: [PATCH RFC v1 3/3] qapi/iothread: introduce poll-weight parameter for aio-poll
Posted by Markus Armbruster 3 weeks, 4 days ago
Jaehoon Kim <jhkim@linux.ibm.com> writes:

> Introduce a new poll-weight parameter for aio-poll. This parameter
> controls how much the most recent event interval affects the next
> polling duration. When set to 0, a default value of 2 is used, meaning
> the current interval contributes roughly 25% to the calculation. Larger
> values decrease the weight of the current interval, enabling more
> gradual adjustments to polling duration.
>
> Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>

[...]

> diff --git a/qapi/misc.json b/qapi/misc.json
> index 28c641fe2f..b21cc48a03 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -85,6 +85,11 @@
>  # @poll-shrink: how many ns will be removed from polling time, 0 means
>  #     that it's not configured (since 2.9)
>  #
> +# @poll-weight: the weight factor for adaptive polling.
> +#     Determines how much the current event interval contributes to
> +#     the next polling time calculation.  0 means that the default
> +#     value is used.  (since 10.1)

When the default value is used, the actual value being used remains
hidden.  Why?

> +#
>  # @aio-max-batch: maximum number of requests in a batch for the AIO
>  #     engine, 0 means that the engine will use its default (since 6.1)
>  #
> @@ -96,6 +101,7 @@
>             'poll-max-ns': 'int',
>             'poll-grow': 'int',
>             'poll-shrink': 'int',
> +           'poll-weight': 'int',
>             'aio-max-batch': 'int' } }
>  
>  ##
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 6f5c9de0f0..d90823478d 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -606,6 +606,11 @@
>  #     algorithm detects it is spending too long polling without
>  #     encountering events.  0 selects a default behaviour (default: 0)
>  #
> +# @poll-weight: the weight factor for adaptive polling.
> +#     Determines how much the current event interval contributes to
> +#     the next polling time calculation.  0 selects a default
> +#     behaviour (default: 0) since 10.1.

This leaves the actual default behavior unspecified.  Is this a good
idea?

> +#
>  # The @aio-max-batch option is available since 6.1.
>  #
>  # Since: 2.0
> @@ -614,7 +619,8 @@
>    'base': 'EventLoopBaseProperties',
>    'data': { '*poll-max-ns': 'int',
>              '*poll-grow': 'int',
> -            '*poll-shrink': 'int' } }
> +            '*poll-shrink': 'int',
> +            '*poll-weight': 'int' } }
>  
>  ##
>  # @MainLoopProperties:

[...]
Re: [PATCH RFC v1 3/3] qapi/iothread: introduce poll-weight parameter for aio-poll
Posted by JAEHOON KIM 3 weeks, 3 days ago
On 1/14/2026 1:48 AM, Markus Armbruster wrote:
> Jaehoon Kim <jhkim@linux.ibm.com> writes:
>
>> Introduce a new poll-weight parameter for aio-poll. This parameter
>> controls how much the most recent event interval affects the next
>> polling duration. When set to 0, a default value of 2 is used, meaning
>> the current interval contributes roughly 25% to the calculation. Larger
>> values decrease the weight of the current interval, enabling more
>> gradual adjustments to polling duration.
>>
>> Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
> [...]
>
>> diff --git a/qapi/misc.json b/qapi/misc.json
>> index 28c641fe2f..b21cc48a03 100644
>> --- a/qapi/misc.json
>> +++ b/qapi/misc.json
>> @@ -85,6 +85,11 @@
>>   # @poll-shrink: how many ns will be removed from polling time, 0 means
>>   #     that it's not configured (since 2.9)
>>   #
>> +# @poll-weight: the weight factor for adaptive polling.
>> +#     Determines how much the current event interval contributes to
>> +#     the next polling time calculation.  0 means that the default
>> +#     value is used.  (since 10.1)
> When the default value is used, the actual value being used remains
> hidden.  Why?
Actually, I just followed the existing pattern of poll-grow, which also 
defaults to a factor of 2 when set to 0.
It wasn't my intention to hide the value; I kept this because the 
previous API has been working fine without issues.
If you think the actual value should be visible, I'll consider ways to 
make it explicit in the next version.
>> +#
>>   # @aio-max-batch: maximum number of requests in a batch for the AIO
>>   #     engine, 0 means that the engine will use its default (since 6.1)
>>   #
>> @@ -96,6 +101,7 @@
>>              'poll-max-ns': 'int',
>>              'poll-grow': 'int',
>>              'poll-shrink': 'int',
>> +           'poll-weight': 'int',
>>              'aio-max-batch': 'int' } }
>>   
>>   ##
>> diff --git a/qapi/qom.json b/qapi/qom.json
>> index 6f5c9de0f0..d90823478d 100644
>> --- a/qapi/qom.json
>> +++ b/qapi/qom.json
>> @@ -606,6 +606,11 @@
>>   #     algorithm detects it is spending too long polling without
>>   #     encountering events.  0 selects a default behaviour (default: 0)
>>   #
>> +# @poll-weight: the weight factor for adaptive polling.
>> +#     Determines how much the current event interval contributes to
>> +#     the next polling time calculation.  0 selects a default
>> +#     behaviour (default: 0) since 10.1.
> This leaves the actual default behavior unspecified.  Is this a good
> idea?
I agree that the documentation should be more explicit.
I'll update it to clarify that the default factor is 2 and explain its 
meaning.
>> +#
>>   # The @aio-max-batch option is available since 6.1.
>>   #
>>   # Since: 2.0
>> @@ -614,7 +619,8 @@
>>     'base': 'EventLoopBaseProperties',
>>     'data': { '*poll-max-ns': 'int',
>>               '*poll-grow': 'int',
>> -            '*poll-shrink': 'int' } }
>> +            '*poll-shrink': 'int',
>> +            '*poll-weight': 'int' } }
>>   
>>   ##
>>   # @MainLoopProperties:
> [...]
>
>
Re: [PATCH RFC v1 3/3] qapi/iothread: introduce poll-weight parameter for aio-poll
Posted by Markus Armbruster 3 weeks, 3 days ago
Cc: devel@lists.libvirt.org for a possible query-iothreads change
discussed below.

JAEHOON KIM <jhkim@linux.ibm.com> writes:

> On 1/14/2026 1:48 AM, Markus Armbruster wrote:
>> Jaehoon Kim <jhkim@linux.ibm.com> writes:
>>
>>> Introduce a new poll-weight parameter for aio-poll. This parameter
>>> controls how much the most recent event interval affects the next
>>> polling duration. When set to 0, a default value of 2 is used, meaning
>>> the current interval contributes roughly 25% to the calculation. Larger
>>> values decrease the weight of the current interval, enabling more
>>> gradual adjustments to polling duration.
>>>
>>> Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
>> [...]
>>
>>> diff --git a/qapi/misc.json b/qapi/misc.json
>>> index 28c641fe2f..b21cc48a03 100644
>>> --- a/qapi/misc.json
>>> +++ b/qapi/misc.json
>>> @@ -85,6 +85,11 @@
>>>  # @poll-shrink: how many ns will be removed from polling time, 0 means
>>>  #     that it's not configured (since 2.9)
>>>  #
>>> +# @poll-weight: the weight factor for adaptive polling.
>>> +#     Determines how much the current event interval contributes to
>>> +#     the next polling time calculation.  0 means that the default
>>> +#     value is used.  (since 10.1)
>>
>> When the default value is used, the actual value being used remains
>> hidden.  Why?
>
> Actually, I just followed the existing pattern of poll-grow, which also 
> defaults to a factor of 2 when set to 0.

Yes, and consistency is always desirable.  But let's have a look at the
new interface in isolation, to see whether it's actually good.

> It wasn't my intention to hide the value; I kept this because the 
> previous API has been working fine without issues.
> If you think the actual value should be visible, I'll consider ways to 
> make it explicit in the next version.

As is, query-iothreads tells us "the weight factor for adaptive polling
is X, and it was set by the user", or "the weight factor for adaptive
polling was not set by the user, but picked by the system."

If we returned the actual value, it would tell us "the weight factor for
adaptive polling is X".  

Only the former interface tells us whether the user or the system
picked.

Only the latter interface tells us what the system picked.

Which one is useful in practice?

I'd argue the latter.  A management application knows whether it set a
value without query-iothreads' help, but it doesn't know what the system
picked.  The people coding it may know if a contract specifies what the
system picks (see below).

If we conclude that returning the actual value is better for new
@poll-weight, then it would surely be better for @poll-grow and
@poll-shrink, too.  Could we still improve them?

Libvirt developers, any advice?

>>> +#
>>>  # @aio-max-batch: maximum number of requests in a batch for the AIO
>>>  #     engine, 0 means that the engine will use its default (since 6.1)
>>>  #
>>> @@ -96,6 +101,7 @@
>>>              'poll-max-ns': 'int',
>>>              'poll-grow': 'int',
>>>              'poll-shrink': 'int',
>>> +           'poll-weight': 'int',
>>>              'aio-max-batch': 'int' } }
>>>   
>>>  ##
>>> diff --git a/qapi/qom.json b/qapi/qom.json
>>> index 6f5c9de0f0..d90823478d 100644
>>> --- a/qapi/qom.json
>>> +++ b/qapi/qom.json
>>> @@ -606,6 +606,11 @@
>>>  #     algorithm detects it is spending too long polling without
>>>  #     encountering events.  0 selects a default behaviour (default: 0)
>>>  #
>>> +# @poll-weight: the weight factor for adaptive polling.
>>> +#     Determines how much the current event interval contributes to
>>> +#     the next polling time calculation.  0 selects a default
>>> +#     behaviour (default: 0) since 10.1.
>>
>> This leaves the actual default behavior unspecified.  Is this a good
>> idea?
>
> I agree that the documentation should be more explicit.
> I'll update it to clarify that the default factor is 2 and explain its 
> meaning.

I understand that you're mirroring how @poll-grow and @poll-shrink work,
but let's ignore that for a minute.

Compare four possible interfaces:

1. Optional @poll-weight defaults to 2.  Values <= 0 are rejected.

2. Optional @poll-weight defaults to 2.  Value 0 is replaced by the
   default value 2.  Values < 0 are rejected.

3. Optional @poll-weight defaults to 0.  Values < 0 are rejected.  Value
   0 makes the system pick a value, namely 2.

4. Optional @poll-weight defaults to 0.  Values < 0 are rejected.  Value
   0 makes the system pick a value.  It currently picks 2.

The difference between 3. and 4. is that 3. makes "system picks 2" part
of the contract, while 4. doesn't.

1. is the simplest.  Is 2.'s additional complexity worthwhile?  3.'s?
4.'s?

>>> +#
>>>  # The @aio-max-batch option is available since 6.1.
>>>  #
>>>  # Since: 2.0
>>> @@ -614,7 +619,8 @@
>>>     'base': 'EventLoopBaseProperties',
>>>     'data': { '*poll-max-ns': 'int',
>>>               '*poll-grow': 'int',
>>> -            '*poll-shrink': 'int' } }
>>> +            '*poll-shrink': 'int',
>>> +            '*poll-weight': 'int' } }
>>>   
>>>  ##
>>>  # @MainLoopProperties:
>>
>> [...]
Re: [PATCH RFC v1 3/3] qapi/iothread: introduce poll-weight parameter for aio-poll
Posted by Halil Pasic 3 weeks, 3 days ago
On Thu, 15 Jan 2026 08:28:51 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> I understand that you're mirroring how @poll-grow and @poll-shrink work,
> but let's ignore that for a minute.
> 
> Compare four possible interfaces:
> 
> 1. Optional @poll-weight defaults to 2.  Values <= 0 are rejected.
> 
> 2. Optional @poll-weight defaults to 2.  Value 0 is replaced by the
>    default value 2.  Values < 0 are rejected.
> 
> 3. Optional @poll-weight defaults to 0.  Values < 0 are rejected.  Value
>    0 makes the system pick a value, namely 2.
> 
> 4. Optional @poll-weight defaults to 0.  Values < 0 are rejected.  Value
>    0 makes the system pick a value.  It currently picks 2.
> 
> The difference between 3. and 4. is that 3. makes "system picks 2" part
> of the contract, while 4. doesn't.
> 
> 1. is the simplest.  Is 2.'s additional complexity worthwhile?  3.'s?
> 4.'s?

Isn't there more options? Like

5. Optional @poll-weight defaults to system-default.  Value 0 is replaced
by the system pick the system default value. Currently the system default
value is 2. Values < 0 are rejected.

That would mean:
* current value inspectable
* system default not part of the interface contract
* interface offers a "please go back to value not user specified:
  operation

BTW I like your approach with explicitly listing and evaluating the
options a lot!

Regards,
Halil
Re: [PATCH RFC v1 3/3] qapi/iothread: introduce poll-weight parameter for aio-poll
Posted by Markus Armbruster 3 weeks, 2 days ago
Halil Pasic <pasic@linux.ibm.com> writes:

> On Thu, 15 Jan 2026 08:28:51 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> I understand that you're mirroring how @poll-grow and @poll-shrink work,
>> but let's ignore that for a minute.
>> 
>> Compare four possible interfaces:
>> 
>> 1. Optional @poll-weight defaults to 2.  Values <= 0 are rejected.
>> 
>> 2. Optional @poll-weight defaults to 2.  Value 0 is replaced by the
>>    default value 2.  Values < 0 are rejected.
>> 
>> 3. Optional @poll-weight defaults to 0.  Values < 0 are rejected.  Value
>>    0 makes the system pick a value, namely 2.
>> 
>> 4. Optional @poll-weight defaults to 0.  Values < 0 are rejected.  Value
>>    0 makes the system pick a value.  It currently picks 2.
>> 
>> The difference between 3. and 4. is that 3. makes "system picks 2" part
>> of the contract, while 4. doesn't.
>> 
>> 1. is the simplest.  Is 2.'s additional complexity worthwhile?  3.'s?
>> 4.'s?
>
> Isn't there more options? Like

Yes :)

> 5. Optional @poll-weight defaults to system-default.  Value 0 is replaced
> by the system pick the system default value. Currently the system default
> value is 2. Values < 0 are rejected.
>
> That would mean:
> * current value inspectable
> * system default not part of the interface contract
> * interface offers a "please go back to value not user specified:
>   operation
>
> BTW I like your approach with explicitly listing and evaluating the
> options a lot!

Thanks!
Re: [PATCH RFC v1 3/3] qapi/iothread: introduce poll-weight parameter for aio-poll
Posted by JAEHOON KIM 3 weeks, 3 days ago
On 1/15/2026 4:05 AM, Halil Pasic wrote:
> On Thu, 15 Jan 2026 08:28:51 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> I understand that you're mirroring how @poll-grow and @poll-shrink work,
>> but let's ignore that for a minute.
>>
>> Compare four possible interfaces:
>>
>> 1. Optional @poll-weight defaults to 2.  Values <= 0 are rejected.
>>
>> 2. Optional @poll-weight defaults to 2.  Value 0 is replaced by the
>>     default value 2.  Values < 0 are rejected.
>>
>> 3. Optional @poll-weight defaults to 0.  Values < 0 are rejected.  Value
>>     0 makes the system pick a value, namely 2.
>>
>> 4. Optional @poll-weight defaults to 0.  Values < 0 are rejected.  Value
>>     0 makes the system pick a value.  It currently picks 2.
>>
>> The difference between 3. and 4. is that 3. makes "system picks 2" part
>> of the contract, while 4. doesn't.
>>
>> 1. is the simplest.  Is 2.'s additional complexity worthwhile?  3.'s?
>> 4.'s?
> Isn't there more options? Like
>
> 5. Optional @poll-weight defaults to system-default.  Value 0 is replaced
> by the system pick the system default value. Currently the system default
> value is 2. Values < 0 are rejected.
>
> That would mean:
> * current value inspectable
> * system default not part of the interface contract
> * interface offers a "please go back to value not user specified:
>    operation
>
> BTW I like your approach with explicitly listing and evaluating the
> options a lot!
>
> Regards,
> Halil
>
Thank you both for laying out options 1-5 so clearly; the detailed 
breakdown was very helpful.
After considering the trade-offs, I agree that "Option 1" is the 
simplest and most robust interface. It ensures the value exposed to 
users always reflects the actual effect.
Option 5 is a clever way to reset values, but I'm leaning toward Option 
1 to keep the interface as predictable as possible.
Avoiding special meanings for '0' makes the logic easier for users to 
reason about.

I will update the next revision to follow "Option 1".
Thanks again for the feedback!