Re: [Qemu-devel] [v8 08/14] migration: Add the core code of multi-thread compression

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> 
> 
> On 15/04/2015 11:26, Liang Li wrote:
> > +    if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
> > +        if (ret != RAM_SAVE_CONTROL_DELAYED) {
> > +            if (bytes_xmit > 0) {
> > +                acct_info.norm_pages++;
> 
> I don't think you can mix non-atomic and atomic increments like
> this---or if you can, you really should document why.
> 
> Perhaps you can add a counter to the CompressParam struct, and sum all
> counters in norm_mig_pages_transferred/norm_mig_bytes_transferred (the
> latter probably should just call norm_mig_pages_transferred)?

The 'ram_save_compressed_page' that Liang Li has added here is basically
the same as the ram_save_page we've already got but with the extra
bits for compression, and this non-atomic inc is in the code simply copied
to handle the 'ram_control_save_page' case (i.e. RDMA).

So it is safe, because I don't think any pages will get handed to the 
compression threads (and hence hit the atomic inc's) if RDMA is hooking
the ram_control_save_page.

It's a good question what will happen with RDAM+compression enabled;
if we're lucky it'll just do uncompressed-RDMA.

Dave

> 
> Paolo
> 
> > +            } else if (bytes_xmit == 0) {
> > +                acct_info.dup_pages++;
> > +            }
> > +        }
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK