[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v6 08/14] migration: Add the core code of multi-thread
From: |
Li, Liang Z |
Subject: |
Re: [Qemu-devel] [v6 08/14] migration: Add the core code of multi-thread compression |
Date: |
Sat, 28 Mar 2015 06:11:16 +0000 |
> -
> > 1 file changed, 177 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch_init.c b/arch_init.c index 48cae22..9f63c0f 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c
> > @@ -355,12 +355,33 @@ static DecompressParam *decomp_param; static
> > QemuThread *decompress_threads; static uint8_t
> *compressed_data_buf;
> >
> > +static int do_compress_ram_page(CompressParam *param);
> > +
> > static void *do_data_compress(void *opaque) {
> > - while (!quit_comp_thread) {
> > + CompressParam *param = opaque;
> >
> > - /* To be done */
> Change this line to
>
> > + while (!quit_comp_thread) {
>
> while(true) {
>
> > + qemu_mutex_lock(¶m->mutex);
> > + /* Re-check the quit_comp_thread in case of
> > + * terminate_compression_threads is called just before
> > + * qemu_mutex_lock(¶m->mutex) and after
> > + * while(!quit_comp_thread), re-check it here can make
> > + * sure the compression thread terminate as expected.
> > + */
> Change this
>
> > + while (!param->start && !quit_comp_thread) {
>
> to
>
> while (!param->start && !parm->quit) {
>
> > + qemu_cond_wait(¶m->cond, ¶m->mutex);
> > + }
>
> And this
>
> > + if (!quit_comp_thread) {
>
> to
>
> if (!param->quit) {
> > + do_compress_ram_page(param);
> > + }
>
> Take care here of exiting correctly of the loop.
> Notice that the only case where we are not going to take the look is the last
> iteration, so I think the optimization don't gives us nothing (in this
> place), no?
>
> > + param->start = false;
> > + qemu_mutex_unlock(¶m->mutex);
> >
> > + qemu_mutex_lock(comp_done_lock);
> > + param->done = true;
> > + qemu_cond_signal(comp_done_cond);
> > + qemu_mutex_unlock(comp_done_lock);
> > }
>
>
> >
> > return NULL;
> > @@ -368,9 +389,15 @@ static void *do_data_compress(void *opaque)
> >
> > static inline void terminate_compression_threads(void)
> > {
> > - quit_comp_thread = true;
> > + int idx, thread_count;
> >
> > - /* To be done */
> > + thread_count = migrate_compress_threads();
> > + quit_comp_thread = true;
>
>
> > + for (idx = 0; idx < thread_count; idx++) {
> > + qemu_mutex_lock(&comp_param[idx].mutex);
> Add this
> comp_param[idx].quit = true;
>
>
> And for now on, quit_comp_thread is only used on migration_thread, so it
> should be safe to use, no?
>
> flush_compresed_data() is only ever called from the migration_thread, so no
> lock there needed either.
Now that the lock is no needed in flush_comrepssed_data(), it looks good to me.
I will change the code according to your suggestion.
And could you ask the related maintainers to review the other parts of my
patches,
especially the 3 patches related to the qmp and hmp interfaces.
Thanks Juan!
Liang
[Qemu-devel] [v6 03/14] migration: Add the framework of multi-thread decompression, Liang Li, 2015/03/23
[Qemu-devel] [v6 11/14] migration: Add interface to control compression, Liang Li, 2015/03/23
[Qemu-devel] [v6 04/14] qemu-file: Add compression functions to QEMUFile, Liang Li, 2015/03/23
[Qemu-devel] [v6 13/14] migration: Add qmp commands to set and query parameters, Liang Li, 2015/03/23
[Qemu-devel] [v6 10/14] migration: Add the core code for decompression, Liang Li, 2015/03/23
[Qemu-devel] [v6 09/14] migration: Make compression co-work with xbzrle, Liang Li, 2015/03/23
[Qemu-devel] [v6 14/14] migration: Add hmp interface to set and query parameters, Liang Li, 2015/03/23
[Qemu-devel] [v6 06/14] arch_init: Add and free data struct for decompression, Liang Li, 2015/03/23