* Paolo Bonzini (pbonz...@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 / dgilb...@redhat.com / Manchester, UK
