Daniel P. Berrangé <berrange@redhat.com> pointed out that the coroutine
pool size heuristic is very conservative. Instead of halving
max_map_count, he suggested reserving 5,000 mappings for non-coroutine
users based on observations of guests he has access to.

Fixes: 86a637e48104 ("coroutine: cap per-thread local pool size")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/qemu-coroutine.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index 2790959eaf..eb4eebefdf 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -377,12 +377,17 @@ static unsigned int get_global_pool_hard_max_size(void)
                             NULL) &&
         qemu_strtoi(contents, NULL, 10, &max_map_count) == 0) {
         /*
-         * This is a conservative upper bound that avoids exceeding
-         * max_map_count. Leave half for non-coroutine users like library
-         * dependencies, vhost-user, etc. Each coroutine takes up 2 VMAs so
-         * halve the amount again.
+         * This is an upper bound that avoids exceeding max_map_count. Leave a
+         * fixed amount for non-coroutine users like library dependencies,
+         * vhost-user, etc. Each coroutine takes up 2 VMAs so halve the
+         * remaining amount.
          */
-        return max_map_count / 4;
+        if (max_map_count > 5000) {
+            return (max_map_count - 5000) / 2;
+        } else {
+            /* Disable the global pool but threads still have local pools */
+            return 0;
+        }
     }
 #endif
 
-- 
2.44.0

On Wed, Mar 20, 2024 at 02:12:32PM -0400, Stefan Hajnoczi wrote:
> Daniel P. Berrangé <berrange@redhat.com> pointed out that the coroutine
> pool size heuristic is very conservative. Instead of halving
> max_map_count, he suggested reserving 5,000 mappings for non-coroutine
> users based on observations of guests he has access to.
> 
> Fixes: 86a637e48104 ("coroutine: cap per-thread local pool size")
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  util/qemu-coroutine.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)

Discussed with Kevin and applied to my block tree:
https://gitlab.com/stefanha/qemu/commits/block

Stefan

On Wed, Mar 20, 2024 at 02:12:32PM -0400, Stefan Hajnoczi wrote:
> Daniel P. Berrangé <berrange@redhat.com> pointed out that the coroutine
> pool size heuristic is very conservative. Instead of halving
> max_map_count, he suggested reserving 5,000 mappings for non-coroutine
> users based on observations of guests he has access to.
> 
> Fixes: 86a637e48104 ("coroutine: cap per-thread local pool size")

It wasn't really broken, so "Tweaks" or "Enhances" rather than
"Fixes" if you like :-)

> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  util/qemu-coroutine.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
> index 2790959eaf..eb4eebefdf 100644
> --- a/util/qemu-coroutine.c
> +++ b/util/qemu-coroutine.c
> @@ -377,12 +377,17 @@ static unsigned int get_global_pool_hard_max_size(void)
>                              NULL) &&
>          qemu_strtoi(contents, NULL, 10, &max_map_count) == 0) {
>          /*
> -         * This is a conservative upper bound that avoids exceeding
> -         * max_map_count. Leave half for non-coroutine users like library
> -         * dependencies, vhost-user, etc. Each coroutine takes up 2 VMAs so
> -         * halve the amount again.
> +         * This is an upper bound that avoids exceeding max_map_count. Leave a
> +         * fixed amount for non-coroutine users like library dependencies,
> +         * vhost-user, etc. Each coroutine takes up 2 VMAs so halve the
> +         * remaining amount.
>           */
> -        return max_map_count / 4;
> +        if (max_map_count > 5000) {
> +            return (max_map_count - 5000) / 2;
> +        } else {
> +            /* Disable the global pool but threads still have local pools */
> +            return 0;
> +        }
>      }
>  #endif

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|