[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v7 10/14] migration: Add the core code for decompressi
From: |
Li, Liang Z |
Subject: |
Re: [Qemu-devel] [v7 10/14] migration: Add the core code for decompression |
Date: |
Wed, 8 Apr 2015 14:00:04 +0000 |
> > @@ -889,7 +889,6 @@ static inline void
> start_compression(CompressParam *param)
> > qemu_mutex_unlock(¶m->mutex); }
> >
> > -
> > static uint64_t bytes_transferred;
> >
> > static void flush_compressed_data(QEMUFile *f) @@ -1458,8 +1457,28
> @@
> > void ram_handle_compressed(void *host, uint8_t ch, uint64_t size)
> >
> > static void *do_data_decompress(void *opaque) {
> > + DecompressParam *param = opaque;
> > + size_t pagesize;
> > +
> > while (true) {
> > - /* To be done */
> > + qemu_mutex_lock(¶m->mutex);
> > + while (!param->start && !param->quit) {
> > + qemu_cond_wait(¶m->cond, ¶m->mutex);
>
> We don't check here if we have received a quit
>
>
> > + pagesize = TARGET_PAGE_SIZE;
> > + /* uncompress() will return failed in some case, especially
> > + * when the page is dirted when doing the compression, it's
> > + * not a problem because the dirty page will be retransferred
> > + * and uncompress() won't break the data in other pages.
> > + */
> > + uncompress((Bytef *)param->des, &pagesize,
> > + (const Bytef *)param->compbuf, param->len);
> > + param->start = false;
> > + }
> > + if (param->quit) {
> > + qemu_mutex_unlock(¶m->mutex);
> > + break;
> > + }
> > + qemu_mutex_unlock(¶m->mutex);
> > }
> >
> > return NULL;
> > @@ -1489,6 +1508,12 @@ void migrate_decompress_threads_join(void)
> >
> > thread_count = migrate_decompress_threads();
> > for (i = 0; i < thread_count; i++) {
> > + qemu_mutex_lock(&decomp_param[i].mutex);
> > + decomp_param[i].quit = true;
> > + qemu_cond_signal(&decomp_param[i].cond);
>
> We set quit and send a signal to wakeup the thread, but it will try to
> uncompress a page. Shouldn't we change the position of the param->quit
> check for exit? I think it should be inside the loop.
Yes, you are right. if (param->quit) should be put in front of uncompress().
I will change it in the final version. (I really hope the next version is the
final version :) )
- [Qemu-devel] [v7 05/14] arch_init: Alloc and free data struct for compression, (continued)
- [Qemu-devel] [v7 05/14] arch_init: Alloc and free data struct for compression, Liang Li, 2015/04/08
- [Qemu-devel] [v7 06/14] arch_init: Add and free data struct for decompression, Liang Li, 2015/04/08
- [Qemu-devel] [v7 07/14] migration: Split save_zero_page from ram_save_page, Liang Li, 2015/04/08
- [Qemu-devel] [v7 08/14] migration: Add the core code of multi-thread compression, Liang Li, 2015/04/08
- [Qemu-devel] [v7 09/14] migration: Make compression co-work with xbzrle, Liang Li, 2015/04/08
- [Qemu-devel] [v7 10/14] migration: Add the core code for decompression, Liang Li, 2015/04/08
- [Qemu-devel] [v7 11/14] migration: Add interface to control compression, Liang Li, 2015/04/08
- [Qemu-devel] [v7 12/14] migration: Use an array instead of 3 parameters, Liang Li, 2015/04/08