[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v3 05/13] arch_init: alloc and free data struct in mul
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [v3 05/13] arch_init: alloc and free data struct in multi-thread compression |
Date: |
Fri, 23 Jan 2015 13:35:09 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Liang Li (address@hidden) wrote:
> Define the data structure and varibles used when doing multiple
> thread compression, and add the code to initialize and free them.
>
> Signed-off-by: Liang Li <address@hidden>
> Signed-off-by: Yang Zhang <address@hidden>
> ---
> arch_init.c | 34 +++++++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 2f1d0c4..f21a8ea 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -340,16 +340,29 @@ static bool ram_bulk_stage;
> #define COMPRESS_BUF_SIZE (TARGET_PAGE_SIZE + 16)
>
> struct compress_param {
> - /* To be done */
> + int state;
> + QEMUFile *file;
> + QemuMutex mutex;
> + QemuCond cond;
> + RAMBlock *block;
> + ram_addr_t offset;
> };
> typedef struct compress_param compress_param;
>
> +enum {
> + DONE,
> + START,
> +};
> +
Do you really need any more than a 'bool busy' ?
> struct decompress_param {
> /* To be done */
> };
> typedef struct decompress_param decompress_param;
>
> static compress_param *comp_param;
> +static QemuMutex *mutex;
> +static QemuCond *cond;
Those need better names and a comment; If I'm reading it
correctly, this cond is used to wake up the parent thread
when one of the workers has finished it's task?
> +static QEMUFileOps *empty_ops;
> static bool quit_thread;
> static decompress_param *decomp_param;
> static QemuThread *decompress_threads;
> @@ -381,11 +394,22 @@ void migrate_compress_threads_join(MigrationState *s)
> thread_count = migrate_compress_threads();
> for (i = 0; i < thread_count; i++) {
> qemu_thread_join(s->compress_thread + i);
> + qemu_fclose(comp_param[i].file);
> + qemu_mutex_destroy(&comp_param[i].mutex);
> + qemu_cond_destroy(&comp_param[i].cond);
> }
> + qemu_mutex_destroy(mutex);
> + qemu_cond_destroy(cond);
> g_free(s->compress_thread);
> g_free(comp_param);
> + g_free(cond);
> + g_free(mutex);
> + g_free(empty_ops);
> s->compress_thread = NULL;
> comp_param = NULL;
> + cond = NULL;
> + mutex = NULL;
> + empty_ops = NULL;
> }
>
> void migrate_compress_threads_create(MigrationState *s)
> @@ -400,7 +424,15 @@ void migrate_compress_threads_create(MigrationState *s)
> s->compress_thread = g_malloc0(sizeof(QemuThread)
> * thread_count);
> comp_param = g_malloc0(sizeof(compress_param) * thread_count);
> + cond = g_malloc0(sizeof(QemuCond));
> + mutex = g_malloc0(sizeof(QemuMutex));
> + empty_ops = g_malloc0(sizeof(QEMUFileOps));
Again this needs to go with the explanation of what you're using
the special QEMUFile for; but I don't think anything outside of
QEMUFile should be allocating a QEMUFileOps (It could be static anyway
rather than malloc'd). I think you could make empty_ops declared
static in qemu-file.c
> + qemu_cond_init(cond);
> + qemu_mutex_init(mutex);
> for (i = 0; i < thread_count; i++) {
> + comp_param[i].file = qemu_fopen_ops(NULL, empty_ops);
> + qemu_mutex_init(&comp_param[i].mutex);
> + qemu_cond_init(&comp_param[i].cond);
> qemu_thread_create(s->compress_thread + i, "compress",
> do_data_compress, comp_param + i, QEMU_THREAD_JOINABLE);
>
> --
> 1.8.3.1
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- Re: [Qemu-devel] [v3 05/13] arch_init: alloc and free data struct in multi-thread compression,
Dr. David Alan Gilbert <=