[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 3/7] util/oslib-posix: Introduce and use MemsetContext for
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH v4 3/7] util/oslib-posix: Introduce and use MemsetContext for touch_all_pages() |
Date: |
Thu, 7 Oct 2021 11:05:24 +0100 |
User-agent: |
Mutt/2.0.7 (2021-05-04) |
* David Hildenbrand (david@redhat.com) wrote:
> Let's minimize the number of global variables to prepare for
> os_mem_prealloc() getting called concurrently and make the code a bit
> easier to read.
>
> The only consumer that really needs a global variable is the sigbus
> handler, which will require protection via a mutex in the future either way
> as we cannot concurrently mess with the SIGBUS handler.
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> util/oslib-posix.c | 73 +++++++++++++++++++++++++++++-----------------
> 1 file changed, 47 insertions(+), 26 deletions(-)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index cb89e07770..cf2ead54ad 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -73,21 +73,30 @@
>
> #define MAX_MEM_PREALLOC_THREAD_COUNT 16
>
> +struct MemsetThread;
> +
> +typedef struct MemsetContext {
> + bool all_threads_created;
> + bool any_thread_failed;
> + struct MemsetThread *threads;
> + int num_threads;
> +} MemsetContext;
> +
> struct MemsetThread {
> char *addr;
> size_t numpages;
> size_t hpagesize;
> QemuThread pgthread;
> sigjmp_buf env;
> + MemsetContext *context;
> };
> typedef struct MemsetThread MemsetThread;
>
> -static MemsetThread *memset_thread;
> -static int memset_num_threads;
> +/* used by sigbus_handler() */
> +static MemsetContext *sigbus_memset_context;
>
> static QemuMutex page_mutex;
> static QemuCond page_cond;
> -static bool threads_created_flag;
Is there a reason you didn't lift page_mutex and page_cond into the
MemsetContext ?
(You don't need to change it for this series, just a thought;
another thought is that I think we hav ea few threadpools like this
with hooks to check they've all been created and to do something if one
fails).
Dave
> int qemu_get_thread_id(void)
> {
> @@ -438,10 +447,13 @@ const char *qemu_get_exec_dir(void)
> static void sigbus_handler(int signal)
> {
> int i;
> - if (memset_thread) {
> - for (i = 0; i < memset_num_threads; i++) {
> - if (qemu_thread_is_self(&memset_thread[i].pgthread)) {
> - siglongjmp(memset_thread[i].env, 1);
> +
> + if (sigbus_memset_context) {
> + for (i = 0; i < sigbus_memset_context->num_threads; i++) {
> + MemsetThread *thread = &sigbus_memset_context->threads[i];
> +
> + if (qemu_thread_is_self(&thread->pgthread)) {
> + siglongjmp(thread->env, 1);
> }
> }
> }
> @@ -459,7 +471,7 @@ static void *do_touch_pages(void *arg)
> * clearing until all threads have been created.
> */
> qemu_mutex_lock(&page_mutex);
> - while(!threads_created_flag){
> + while (!memset_args->context->all_threads_created) {
> qemu_cond_wait(&page_cond, &page_mutex);
> }
> qemu_mutex_unlock(&page_mutex);
> @@ -502,7 +514,7 @@ static void *do_madv_populate_write_pages(void *arg)
>
> /* See do_touch_pages(). */
> qemu_mutex_lock(&page_mutex);
> - while (!threads_created_flag) {
> + while (!memset_args->context->all_threads_created) {
> qemu_cond_wait(&page_cond, &page_mutex);
> }
> qemu_mutex_unlock(&page_mutex);
> @@ -529,6 +541,9 @@ static int touch_all_pages(char *area, size_t hpagesize,
> size_t numpages,
> int smp_cpus, bool use_madv_populate_write)
> {
> static gsize initialized = 0;
> + MemsetContext context = {
> + .num_threads = get_memset_num_threads(smp_cpus),
> + };
> size_t numpages_per_thread, leftover;
> void *(*touch_fn)(void *);
> int ret = 0, i = 0;
> @@ -546,35 +561,41 @@ static int touch_all_pages(char *area, size_t
> hpagesize, size_t numpages,
> touch_fn = do_touch_pages;
> }
>
> - threads_created_flag = false;
> - memset_num_threads = get_memset_num_threads(smp_cpus);
> - memset_thread = g_new0(MemsetThread, memset_num_threads);
> - numpages_per_thread = numpages / memset_num_threads;
> - leftover = numpages % memset_num_threads;
> - for (i = 0; i < memset_num_threads; i++) {
> - memset_thread[i].addr = addr;
> - memset_thread[i].numpages = numpages_per_thread + (i < leftover);
> - memset_thread[i].hpagesize = hpagesize;
> - qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
> - touch_fn, &memset_thread[i],
> + context.threads = g_new0(MemsetThread, context.num_threads);
> + numpages_per_thread = numpages / context.num_threads;
> + leftover = numpages % context.num_threads;
> + for (i = 0; i < context.num_threads; i++) {
> + context.threads[i].addr = addr;
> + context.threads[i].numpages = numpages_per_thread + (i < leftover);
> + context.threads[i].hpagesize = hpagesize;
> + context.threads[i].context = &context;
> + qemu_thread_create(&context.threads[i].pgthread, "touch_pages",
> + touch_fn, &context.threads[i],
> QEMU_THREAD_JOINABLE);
> - addr += memset_thread[i].numpages * hpagesize;
> + addr += context.threads[i].numpages * hpagesize;
> + }
> +
> + if (!use_madv_populate_write) {
> + sigbus_memset_context = &context;
> }
>
> qemu_mutex_lock(&page_mutex);
> - threads_created_flag = true;
> + context.all_threads_created = true;
> qemu_cond_broadcast(&page_cond);
> qemu_mutex_unlock(&page_mutex);
>
> - for (i = 0; i < memset_num_threads; i++) {
> - int tmp = (uintptr_t)qemu_thread_join(&memset_thread[i].pgthread);
> + for (i = 0; i < context.num_threads; i++) {
> + int tmp = (uintptr_t)qemu_thread_join(&context.threads[i].pgthread);
>
> if (tmp) {
> ret = tmp;
> }
> }
> - g_free(memset_thread);
> - memset_thread = NULL;
> +
> + if (!use_madv_populate_write) {
> + sigbus_memset_context = NULL;
> + }
> + g_free(context.threads);
>
> return ret;
> }
> --
> 2.31.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
- [PATCH v4 0/7] util/oslib-posix: Support MADV_POPULATE_WRITE for os_mem_prealloc(), David Hildenbrand, 2021/10/04
- [PATCH v4 1/7] util/oslib-posix: Let touch_all_pages() return an error, David Hildenbrand, 2021/10/04
- [PATCH v4 2/7] util/oslib-posix: Support MADV_POPULATE_WRITE for os_mem_prealloc(), David Hildenbrand, 2021/10/04
- [PATCH v4 4/7] util/oslib-posix: Don't create too many threads with small memory or little pages, David Hildenbrand, 2021/10/04
- [PATCH v4 6/7] util/oslib-posix: Support concurrent os_mem_prealloc() invocation, David Hildenbrand, 2021/10/04
- [PATCH v4 3/7] util/oslib-posix: Introduce and use MemsetContext for touch_all_pages(), David Hildenbrand, 2021/10/04
- Re: [PATCH v4 3/7] util/oslib-posix: Introduce and use MemsetContext for touch_all_pages(),
Dr. David Alan Gilbert <=
- [PATCH v4 7/7] util/oslib-posix: Forward SIGBUS to MCE handler under Linux, David Hildenbrand, 2021/10/04
- [PATCH v4 5/7] util/oslib-posix: Avoid creating a single thread with MADV_POPULATE_WRITE, David Hildenbrand, 2021/10/04
- Re: [PATCH v4 0/7] util/oslib-posix: Support MADV_POPULATE_WRITE for os_mem_prealloc(), David Hildenbrand, 2021/10/11