On 14 March 2017 at 16:18, Paolo Bonzini <[email protected]> wrote:
> From: Jitendra Kolhe <[email protected]>
>
> Using "-mem-prealloc" option for a large guest leads to higher guest
> start-up and migration time. This is because with "-mem-prealloc" option
> qemu tries to map every guest page (create address translations), and
> make sure the pages are available during runtime. virsh/libvirt by
> default, seems to use "-mem-prealloc" option in case the guest is
> configured to use huge pages. The patch tries to map all guest pages
> simultaneously by spawning multiple threads. Currently limiting the
> change to QEMU library functions on POSIX compliant host only, as we are
> not sure if the problem exists on win32. Below are some stats with
> "-mem-prealloc" option for guest configured to use huge pages.
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -55,6 +55,21 @@
> #include "qemu/error-report.h"
> #endif
>
> +#define MAX_MEM_PREALLOC_THREAD_COUNT (MIN(sysconf(_SC_NPROCESSORS_ONLN),
> 16))
sysconf() can fail, in which case it will return -1...
> +static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
> + int smp_cpus)
> +{
> + uint64_t numpages_per_thread, size_per_thread;
> + char *addr = area;
> + int i = 0;
> +
> + memset_thread_failed = false;
> + memset_num_threads = MIN(smp_cpus, MAX_MEM_PREALLOC_THREAD_COUNT);
...and memset_num_threads (being signed) will also end up -1 here...
> + memset_thread = g_new0(MemsetThread, memset_num_threads);
...which we then pass to g_new0(), which will not have good
results.
(Spotted by Coverity, CID 1372465.)
Hiding a sysconf() behind a macro that looks like it's going
to be a constant integer is a bit confusing, incidentally,
and we only use it in one place. I'd just have a memset_num_threads()
function which calls sysconf and determines the right number from
that and smp_cpus and 16, and handles sysconf failing gracefully.
thanks
-- PMM
