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

[Xiao Guangrong]

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.