Re: [Qemu-devel] [PATCH v3 3/5] migration: use threaded workqueue for compression

[Xiao Guangrong <guangrong.xiao@gmail.com>]

On 11/24/18 2:29 AM, Dr. David Alan Gilbert wrote:

>>>>   static void
>>>> -update_compress_thread_counts(const CompressParam *param, int bytes_xmit)
>>>> +update_compress_thread_counts(CompressData *cd, int bytes_xmit)
>>>
>>> Keep the const?

Yes, indeed. Will correct it in the next version.

>>>> +    if (deflateInit(&cd->stream, migrate_compress_level()) != Z_OK) {
>>>> +        g_free(cd->originbuf);
>>>> +        return -1;
>>>> +    }
>>>
>>> Please print errors if you fail in any case so we can easily tell what
>>> happened.

Sure, will do.

>>>> +        if (wait) {
>>>> +            cpu_relax();
>>>> +            goto retry;
>>>
>>> Is there nothing better we can use to wait without eating CPU time?
>>
>> There is a mechanism to wait without eating CPU time in the data
>> structure, but it makes sense to busy wait.  There are 4 threads in the
>> workqueue, so you have to compare 1/4th of the time spent compressing a
>> page, with the trip into the kernel to wake you up.  You're adding 20%
>> CPU usage, but I'm not surprised it's worthwhile.
> 
> Hmm OK; in that case it does at least need a comment because it's a bit
> odd, and we should watch out how that scales - I guess it's less of
> an overhead the more threads you use.
> 

Sure, will add some comments to explain the purpose.