From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51768) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEeOO-0001Qk-7n for qemu-devel@nongnu.org; Fri, 23 Jan 2015 08:35:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YEeOJ-0004BN-90 for qemu-devel@nongnu.org; Fri, 23 Jan 2015 08:35:20 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43537) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEeOJ-0004BH-2r for qemu-devel@nongnu.org; Fri, 23 Jan 2015 08:35:15 -0500 Date: Fri, 23 Jan 2015 13:35:09 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20150123133508.GF2370@work-vm> References: <1418347746-15829-1-git-send-email-liang.z.li@intel.com> <1418347746-15829-6-git-send-email-liang.z.li@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1418347746-15829-6-git-send-email-liang.z.li@intel.com> Subject: Re: [Qemu-devel] [v3 05/13] arch_init: alloc and free data struct in multi-thread compression List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liang Li Cc: quintela@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, lcapitulino@redhat.com, yang.z.zhang@intel.com, dgilbert@redhat.com * Liang Li (liang.z.li@intel.com) 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 > Signed-off-by: Yang Zhang > --- > 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 / dgilbert@redhat.com / Manchester, UK