Re: [Qemu-devel] [PATCH 2/8] migration: stop allocating and freeing memory frequently

[Xiao Guangrong]

On 03/15/2018 07:03 PM, Dr. David Alan Gilbert wrote:

> > +static int compress_threads_load_setup(void)
> > +{
> > +    int i, thread_count;
> > +
> > +    if (!migrate_use_compression()) {
> > +        return 0;
> > +    }
> > +
> > +    thread_count = migrate_decompress_threads();
> > +    decompress_threads = g_new0(QemuThread, thread_count);
> > +    decomp_param = g_new0(DecompressParam, thread_count);
> > +    qemu_mutex_init(&decomp_done_lock);
> > +    qemu_cond_init(&decomp_done_cond);
> > +    for (i = 0; i < thread_count; i++) {
> > +        if (inflateInit(&decomp_param[i].stream) != Z_OK) {
> > +            goto exit;
> > +        }
> > +        decomp_param[i].stream.opaque = &decomp_param[i];
> > +
> > +        qemu_mutex_init(&decomp_param[i].mutex);
> > +        qemu_cond_init(&decomp_param[i].cond);
> > +        decomp_param[i].compbuf = g_malloc0(compressBound(TARGET_PAGE_SIZE));
> > +        decomp_param[i].done = true;
> > +        decomp_param[i].quit = false;
> > +        qemu_thread_create(decompress_threads + i, "decompress",
> > +                           do_data_decompress, decomp_param + i,
> > +                           QEMU_THREAD_JOINABLE);
> > +    }
> > +    return 0;
> > +exit:
> > +    compress_threads_load_cleanup();
>
> I don't think this is safe; if inflateInit(..) fails in not-the-last
> thread, compress_threads_load_cleanup() will try and destroy all the
> mutex's and condition variables, even though they've not yet all been
> _init'd.

That is exactly why we used 'opaque', please see more below...

> However, other than that I think the patch is OK; a chat with Dan
> Berrange has convinced me this probably doesn't affect the stream
> format, so that's OK.
>
> One thing I would like is a comment as to how the 'opaque' field is
> being used; I don't think I quite understand what you're doing there.

The zlib.h file says that:
"     The opaque value provided by the application will be passed as the first
   parameter for calls of zalloc and zfree.  This can be useful for custom
   memory management.  The compression library attaches no meaning to the
   opaque value."
So we can use it to store our private data.

Here, we use it as a indicator which shows if the thread is properly init'd
or not. If inflateInit() is successful we will set it to non-NULL, otherwise
it is NULL, so that the cleanup path can figure out the first thread failed
to do inflateInit().