From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39741) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YqMZ4-0004o4-4m for qemu-devel@nongnu.org; Thu, 07 May 2015 10:14:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YqMZ1-0001TK-EF for qemu-devel@nongnu.org; Thu, 07 May 2015 10:14:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42319) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YqMZ1-0001Rh-5f for qemu-devel@nongnu.org; Thu, 07 May 2015 10:14:11 -0400 Date: Thu, 7 May 2015 15:13:59 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20150507141359.GG2139@work-vm> References: <1429089983-24644-1-git-send-email-liang.z.li@intel.com> <1429089983-24644-9-git-send-email-liang.z.li@intel.com> <554B6264.8060102@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <554B6264.8060102@redhat.com> Subject: Re: [Qemu-devel] [v8 08/14] migration: Add the core code of multi-thread compression List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: quintela@redhat.com, qemu-devel@nongnu.org, Liang Li , armbru@redhat.com, lcapitulino@redhat.com, amit.shah@redhat.com, yang.z.zhang@intel.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