All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] util/coroutine: Reduce default idle pool max size
@ 2026-10-13 22:00 Wesley Hershberger
  2026-10-13 22:01 ` [PATCH v3 1/2] util/coroutine: trim sysctl's max_map_count newline before parsing Wesley Hershberger
  2026-10-13 22:01 ` [PATCH v3 2/2] util/coroutine: Reduce default idle pool max size Wesley Hershberger
  0 siblings, 2 replies; 4+ messages in thread
From: Wesley Hershberger @ 2026-10-13 22:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Kevin Wolf, Hector Cao, Wesley Hershberger,
	alex.ramirez, Daniel P. Berrangé

Adding Hector's patch since mine depends on it; please let me know if
a different approach to sending this would have been preferred.

Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
---
Changes in v3:
- Use Daniel's suggested implementation
- Clarified comments to drop "LSM"
- #define DEFAULT_MAX_MAP_COUNT
- Link to v2: https://lore.kernel.org/qemu-devel/20261013-set-default-pool-size-v2-0-f4fd6f2a3303@canonical.com

Changes in v2:
- Rebase on Hector's patch
- Link to v1: https://lore.kernel.org/qemu-devel/20261013-set-default-pool-size-v1-1-7be19402af6c@canonical.com

---
Hector Cao (1):
      util/coroutine: trim sysctl's max_map_count newline before parsing

Wesley Hershberger (1):
      util/coroutine: Reduce default idle pool max size

 util/qemu-coroutine.c | 75 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 50 insertions(+), 25 deletions(-)
---
base-commit: 615ece3c406b262996e569f29ea6eed0d81c4d8d
change-id: 20261013-set-default-pool-size-8b27c0fca8e3

Best regards,
-- 
Wesley Hershberger <wesley.hershberger@canonical.com>



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/2] util/coroutine: trim sysctl's max_map_count newline before parsing
  2026-10-13 22:00 [PATCH v3 0/2] util/coroutine: Reduce default idle pool max size Wesley Hershberger
@ 2026-10-13 22:01 ` Wesley Hershberger
  2026-10-13 22:01 ` [PATCH v3 2/2] util/coroutine: Reduce default idle pool max size Wesley Hershberger
  1 sibling, 0 replies; 4+ messages in thread
From: Wesley Hershberger @ 2026-10-13 22:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Kevin Wolf, Hector Cao, Wesley Hershberger,
	alex.ramirez, Daniel P. Berrangé

From: Hector Cao <hector.cao@canonical.com>

Strip trailing whitespace from max_map_count before passing it to
qemu_strtoi(), allowing the strict full-string parse to succeed.
Otherwise, qemu_strtoi() fails to parse the string and returns -22,
making the global cache pool cap size set to UINT_MAX.

Fixes: 86a637e48104 ("coroutine: cap per-thread local pool size")
Signed-off-by: Hector Cao <hector.cao@canonical.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
---
 util/qemu-coroutine.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index d17135f585a..5af61995437 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -374,8 +374,12 @@ static unsigned int get_global_pool_hard_max_size(void)
      * VMAs.
      */
     if (g_file_get_contents("/proc/sys/vm/max_map_count", &contents, NULL,
-                            NULL) &&
-        qemu_strtoi(contents, NULL, 10, &max_map_count) == 0) {
+                            NULL)) {
+        /* contents has trailing newline, remove it before conversion */
+        g_strchomp(contents);
+        if (qemu_strtoi(contents, NULL, 10, &max_map_count) != 0) {
+            return UINT_MAX;
+        }
         /*
          * This is an upper bound that avoids exceeding max_map_count. Leave a
          * fixed amount for non-coroutine users like library dependencies,

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 2/2] util/coroutine: Reduce default idle pool max size
  2026-10-13 22:00 [PATCH v3 0/2] util/coroutine: Reduce default idle pool max size Wesley Hershberger
  2026-10-13 22:01 ` [PATCH v3 1/2] util/coroutine: trim sysctl's max_map_count newline before parsing Wesley Hershberger
@ 2026-10-13 22:01 ` Wesley Hershberger
  2026-10-14  7:50   ` Daniel P. Berrangé
  1 sibling, 1 reply; 4+ messages in thread
From: Wesley Hershberger @ 2026-10-13 22:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Kevin Wolf, Hector Cao, Wesley Hershberger,
	alex.ramirez, Daniel P. Berrangé

If g_file_get_contents fails (AppArmor, etc), use a more conservative
default max size for the idle coroutine pool.

This avoids any possibility of the failure described in 86a637e48104
unless the host kernel is configured with a lower-than-default
vm.max_map_count.

This change is motivated by a discussion of backport risk:
https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2168803/comments/8

Fixes: 86a637e48104 ("coroutine: cap per-thread local pool size")
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
---
 util/qemu-coroutine.c | 75 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 27 deletions(-)

diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index 5af61995437..8ced93144fa 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -361,42 +361,63 @@ void qemu_coroutine_dec_pool_size(unsigned int removing_pool_size)
     global_pool_max_size -= removing_pool_size;
 }
 
-static unsigned int get_global_pool_hard_max_size(void)
-{
 #ifdef __linux__
-    g_autofree char *contents = NULL;
-    int max_map_count;
+/*
+ * include/linux/mm.h
+ */
+#define DEFAULT_MAX_MAP_COUNT 65530
 
-    /*
-     * Linux processes can have up to max_map_count virtual memory areas
-     * (VMAs). mmap(2), mprotect(2), etc fail with ENOMEM beyond this limit. We
-     * must limit the coroutine pool to a safe size to avoid running out of
-     * VMAs.
-     */
-    if (g_file_get_contents("/proc/sys/vm/max_map_count", &contents, NULL,
-                            NULL)) {
+static unsigned int get_max_map_count(void)
+{
+    g_autofree char *contents = NULL;
+    int max_map_count = 0;
+    if (g_file_get_contents("/proc/sys/vm/max_map_count", &contents,
+                            NULL, NULL)) {
         /* contents has trailing newline, remove it before conversion */
         g_strchomp(contents);
-        if (qemu_strtoi(contents, NULL, 10, &max_map_count) != 0) {
-            return UINT_MAX;
-        }
-        /*
-         * 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.
-         */
-        if (max_map_count > 5000) {
-            return (max_map_count - 5000) / 2;
-        } else {
-            /* Disable the global pool but threads still have local pools */
-            return 0;
+        if (qemu_strtoi(contents, NULL, 10, &max_map_count) == 0) {
+            return max_map_count;
         }
     }
-#endif
+    return 0;
+}
 
+/*
+ * Linux processes can have up to max_map_count virtual memory areas
+ * (VMAs). mmap(2), mprotect(2), etc fail with ENOMEM beyond this limit. We
+ * must limit the coroutine pool to a safe size to avoid running out of
+ * VMAs.
+ */
+static unsigned int get_global_pool_hard_max_size(void)
+{
+    /*
+     * Read from /proc might fail due to mandatory access control (AppArmor,
+     * SELinux, etc). Use the kernel's default in that case.
+     */
+    int max_map_count = get_max_map_count();
+    if (!max_map_count) {
+        max_map_count = DEFAULT_MAX_MAP_COUNT;
+    }
+
+    /*
+     * 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.
+     */
+    if (max_map_count > 5000) {
+        return (max_map_count - 5000) / 2;
+    } else {
+        /* Disable the global pool but threads still have local pools */
+        return 0;
+    }
+}
+#else
+static unsigned int get_global_pool_hard_max_size(void)
+{
     return UINT_MAX;
 }
+#endif
 
 static void __attribute__((constructor)) qemu_coroutine_init(void)
 {

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 2/2] util/coroutine: Reduce default idle pool max size
  2026-10-13 22:01 ` [PATCH v3 2/2] util/coroutine: Reduce default idle pool max size Wesley Hershberger
@ 2026-10-14  7:50   ` Daniel P. Berrangé
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel P. Berrangé @ 2026-10-14  7:50 UTC (permalink / raw)
  To: Wesley Hershberger
  Cc: qemu-devel, Stefan Hajnoczi, Kevin Wolf, Hector Cao, alex.ramirez

On Tue, Oct 13, 2026 at 05:01:01PM -0500, Wesley Hershberger wrote:
> If g_file_get_contents fails (AppArmor, etc), use a more conservative
> default max size for the idle coroutine pool.
> 
> This avoids any possibility of the failure described in 86a637e48104
> unless the host kernel is configured with a lower-than-default
> vm.max_map_count.
> 
> This change is motivated by a discussion of backport risk:
> https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2168803/comments/8
> 
> Fixes: 86a637e48104 ("coroutine: cap per-thread local pool size")
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
> ---
>  util/qemu-coroutine.c | 75 ++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 48 insertions(+), 27 deletions(-)
> 
> diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
> index 5af61995437..8ced93144fa 100644
> --- a/util/qemu-coroutine.c
> +++ b/util/qemu-coroutine.c
> @@ -361,42 +361,63 @@ void qemu_coroutine_dec_pool_size(unsigned int removing_pool_size)
>      global_pool_max_size -= removing_pool_size;
>  }
>  
> -static unsigned int get_global_pool_hard_max_size(void)
> -{
>  #ifdef __linux__
> -    g_autofree char *contents = NULL;
> -    int max_map_count;
> +/*
> + * include/linux/mm.h
> + */
> +#define DEFAULT_MAX_MAP_COUNT 65530
>  
> -    /*
> -     * Linux processes can have up to max_map_count virtual memory areas
> -     * (VMAs). mmap(2), mprotect(2), etc fail with ENOMEM beyond this limit. We
> -     * must limit the coroutine pool to a safe size to avoid running out of
> -     * VMAs.
> -     */
> -    if (g_file_get_contents("/proc/sys/vm/max_map_count", &contents, NULL,
> -                            NULL)) {
> +static unsigned int get_max_map_count(void)
> +{
> +    g_autofree char *contents = NULL;
> +    int max_map_count = 0;
> +    if (g_file_get_contents("/proc/sys/vm/max_map_count", &contents,
> +                            NULL, NULL)) {
>          /* contents has trailing newline, remove it before conversion */
>          g_strchomp(contents);
> -        if (qemu_strtoi(contents, NULL, 10, &max_map_count) != 0) {
> -            return UINT_MAX;
> -        }
> -        /*
> -         * 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.
> -         */
> -        if (max_map_count > 5000) {
> -            return (max_map_count - 5000) / 2;
> -        } else {
> -            /* Disable the global pool but threads still have local pools */
> -            return 0;
> +        if (qemu_strtoi(contents, NULL, 10, &max_map_count) == 0) {
> +            return max_map_count;
>          }
>      }
> -#endif
> +    return 0;
> +}

I'm now realizing we could just return DEFAULT_MAX_MAP_COUNT
here and removed the conditional in the caller.

>  
> +/*
> + * Linux processes can have up to max_map_count virtual memory areas
> + * (VMAs). mmap(2), mprotect(2), etc fail with ENOMEM beyond this limit. We
> + * must limit the coroutine pool to a safe size to avoid running out of
> + * VMAs.
> + */
> +static unsigned int get_global_pool_hard_max_size(void)
> +{
> +    /*
> +     * Read from /proc might fail due to mandatory access control (AppArmor,
> +     * SELinux, etc). Use the kernel's default in that case.
> +     */
> +    int max_map_count = get_max_map_count();
> +    if (!max_map_count) {
> +        max_map_count = DEFAULT_MAX_MAP_COUNT;
> +    }
> +
> +    /*
> +     * 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.
> +     */
> +    if (max_map_count > 5000) {
> +        return (max_map_count - 5000) / 2;
> +    } else {
> +        /* Disable the global pool but threads still have local pools */
> +        return 0;
> +    }
> +}
> +#else
> +static unsigned int get_global_pool_hard_max_size(void)
> +{
>      return UINT_MAX;
>  }
> +#endif
>  
>  static void __attribute__((constructor)) qemu_coroutine_init(void)
>  {
> 
> -- 
> 2.53.0
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-14  7:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-13 22:00 [PATCH v3 0/2] util/coroutine: Reduce default idle pool max size Wesley Hershberger
2026-10-13 22:01 ` [PATCH v3 1/2] util/coroutine: trim sysctl's max_map_count newline before parsing Wesley Hershberger
2026-10-13 22:01 ` [PATCH v3 2/2] util/coroutine: Reduce default idle pool max size Wesley Hershberger
2026-10-14  7:50   ` Daniel P. Berrangé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.