From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45514) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLo8o-0002FJ-6f for qemu-devel@nongnu.org; Thu, 12 Feb 2015 02:24:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLo8k-0001kb-Vr for qemu-devel@nongnu.org; Thu, 12 Feb 2015 02:24:50 -0500 Received: from mga01.intel.com ([192.55.52.88]:65278) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLo8k-0001kQ-QH for qemu-devel@nongnu.org; Thu, 12 Feb 2015 02:24:46 -0500 From: "Li, Liang Z" Date: Thu, 12 Feb 2015 07:24:34 +0000 Message-ID: References: <1423623986-590-1-git-send-email-liang.z.li@intel.com> <1423623986-590-3-git-send-email-liang.z.li@intel.com> <87iof8bt5r.fsf@neno.neno> In-Reply-To: <87iof8bt5r.fsf@neno.neno> Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [Qemu-devel] [v5 02/12] migration: Add the framework of multi-thread compression List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "quintela@redhat.com" Cc: "armbru@redhat.com" , "qemu-devel@nongnu.org" , "Zhang, Yang Z" , "amit.shah@redhat.com" , "lcapitulino@redhat.com" , "dgilbert@redhat.com" > Reviewing patch 8, I found that we need to fix some things here. >=20 > > +static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block, > > + ram_addr_t offset, bool > > +last_stage) { > > + int bytes_sent =3D -1; > > + > > + /* To be done*/ > > + > > + return bytes_sent; > > +} >=20 > We have three return values, here, that are not the same that for normal > pages >=20 > 0: this is the 1st page for a particular thread, nothing to sent yet n = > 0: we > are sending the previous compresed page for the choosen > thread >=20 > Notice that the only way that ram_save_page() can return 0 is for xbzrle > when a page has modified but it has exactly the same value that before. >=20 > (it can have been modified twice, +1, -1 or whatever) >=20 > Notice that ram_save_page() can only return 0 (duplicate page) or > 0 (re= al > size written) >=20 > > + > > /* > > * ram_find_and_save_block: Finds a page to send and sends it to f > > * > > @@ -679,7 +751,12 @@ static int ram_find_and_save_block(QEMUFile *f, > bool last_stage) > > ram_bulk_stage =3D false; > > } > > } else { > > - bytes_sent =3D ram_save_page(f, block, offset, last_stage)= ; > > + if (migrate_use_compression()) { > > + bytes_sent =3D ram_save_compressed_page(f, block, offs= et, > > + last_stage); > > + } else { > > + bytes_sent =3D ram_save_page(f, block, offset, last_st= age); > > + } >=20 >=20 > I need more context, this is the corrent code >=20 > } else { > bytes_sent =3D ram_save_page(f, block, offset, last_stage); >=20 > /* if page is unmodified, continue to the next */ > if (bytes_sent > 0) { > last_sent_block =3D block; > break; > } > } >=20 > And we should change to: >=20 > } else if (migrate_use_compression()) { > bytes_sent =3D ram_save_compressed_page(f, block, offset, > last_stage); > last_sent_block =3D block; > break; What happened if ram_save_compressed_page() return 0 ? following the flush= _compressed_data() will be call, The code call still work, but the efficiency is poor. Every time the main t= hread find there is no free compression thread, it has to wait all compression threads finish their job before it c= an start the next round.=20 The effective way is to start compression once there is any free compressio= n thread. Liang > } else { > bytes_sent =3D ram_save_page(f, block, offset, last_stage); >=20 > /* if page is unmodified, continue to the next */ > if (bytes_sent > 0) { > last_sent_block =3D block; > break; > } > } >=20 > This would mean that we don't need to arrange for the zero byte return on > qemu. >=20