[Qemu-devel] [PATCH] mem-prealloc: fix sysconf(_SC_NPROCESSORS_ONLN) failure case.

[Qemu-devel] [PATCH] mem-prealloc: fix sysconf(_SC_NPROCESSORS_ONLN) failure case.

[Jitendra Kolhe]

This was spotted by Coverity, in case where sysconf(_SC_NPROCESSORS_ONLN)
fails and returns -1. This results in memset_num_threads getting set to -1.
Which we then pass to g_new0().
The patch replaces MAX_MEM_PREALLOC_THREAD_COUNT macro with a function call
get_memset_num_threads() to handle sysconf() failure gracefully. In case
sysconf() fails, we fall back to single threaded.

(Spotted by Coverity, CID 1372465.)

Signed-off-by: Jitendra Kolhe <jitendra.kolhe@hpe.com>
---
 util/oslib-posix.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 3fe6089..4d9189e 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -55,7 +55,7 @@
 #include "qemu/error-report.h"
 #endif
 
-#define MAX_MEM_PREALLOC_THREAD_COUNT (MIN(sysconf(_SC_NPROCESSORS_ONLN), 16))
+#define MAX_MEM_PREALLOC_THREAD_COUNT 16
 
 struct MemsetThread {
     char *addr;
@@ -381,6 +381,18 @@ static void *do_touch_pages(void *arg)
     return NULL;
 }
 
+static inline int get_memset_num_threads(int smp_cpus)
+{
+    long host_procs = sysconf(_SC_NPROCESSORS_ONLN);
+    int ret = 1;
+
+    if (host_procs > 0) {
+        ret = MIN(MIN(host_procs, MAX_MEM_PREALLOC_THREAD_COUNT), smp_cpus);
+    }
+    /* In case sysconf() fails, we fall back to single threaded */
+    return ret;
+}
+
 static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
                             int smp_cpus)
 {
@@ -389,7 +401,7 @@ static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
     int i = 0;
 
     memset_thread_failed = false;
-    memset_num_threads = MIN(smp_cpus, MAX_MEM_PREALLOC_THREAD_COUNT);
+    memset_num_threads = get_memset_num_threads(smp_cpus);
     memset_thread = g_new0(MemsetThread, memset_num_threads);
     numpages_per_thread = (numpages / memset_num_threads);
     size_per_thread = (hpagesize * numpages_per_thread);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] mem-prealloc: fix sysconf(_SC_NPROCESSORS_ONLN) failure case.

[Paolo Bonzini]

On 21/03/2017 07:50, Jitendra Kolhe wrote:
> This was spotted by Coverity, in case where sysconf(_SC_NPROCESSORS_ONLN)
> fails and returns -1. This results in memset_num_threads getting set to -1.
> Which we then pass to g_new0().
> The patch replaces MAX_MEM_PREALLOC_THREAD_COUNT macro with a function call
> get_memset_num_threads() to handle sysconf() failure gracefully. In case
> sysconf() fails, we fall back to single threaded.
> 
> (Spotted by Coverity, CID 1372465.)
> 
> Signed-off-by: Jitendra Kolhe <jitendra.kolhe@hpe.com>
> ---
>  util/oslib-posix.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 3fe6089..4d9189e 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -55,7 +55,7 @@
>  #include "qemu/error-report.h"
>  #endif
>  
> -#define MAX_MEM_PREALLOC_THREAD_COUNT (MIN(sysconf(_SC_NPROCESSORS_ONLN), 16))
> +#define MAX_MEM_PREALLOC_THREAD_COUNT 16
>  
>  struct MemsetThread {
>      char *addr;
> @@ -381,6 +381,18 @@ static void *do_touch_pages(void *arg)
>      return NULL;
>  }
>  
> +static inline int get_memset_num_threads(int smp_cpus)
> +{
> +    long host_procs = sysconf(_SC_NPROCESSORS_ONLN);
> +    int ret = 1;
> +
> +    if (host_procs > 0) {
> +        ret = MIN(MIN(host_procs, MAX_MEM_PREALLOC_THREAD_COUNT), smp_cpus);
> +    }
> +    /* In case sysconf() fails, we fall back to single threaded */
> +    return ret;
> +}
> +
>  static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
>                              int smp_cpus)
>  {
> @@ -389,7 +401,7 @@ static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
>      int i = 0;
>  
>      memset_thread_failed = false;
> -    memset_num_threads = MIN(smp_cpus, MAX_MEM_PREALLOC_THREAD_COUNT);
> +    memset_num_threads = get_memset_num_threads(smp_cpus);
>      memset_thread = g_new0(MemsetThread, memset_num_threads);
>      numpages_per_thread = (numpages / memset_num_threads);
>      size_per_thread = (hpagesize * numpages_per_thread);
> 

Queued, thanks.

Paolo