From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52146) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTuDD-0003Cc-7m for qemu-devel@nongnu.org; Sat, 29 Mar 2014 10:26:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WTuD8-0007Zm-8L for qemu-devel@nongnu.org; Sat, 29 Mar 2014 10:26:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:9562) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTuD7-0007Z5-T3 for qemu-devel@nongnu.org; Sat, 29 Mar 2014 10:26:14 -0400 Message-ID: <5336D7FE.5050904@redhat.com> Date: Sat, 29 Mar 2014 08:26:06 -0600 From: Eric Blake MIME-Version: 1.0 References: <1396079541-7112-1-git-send-email-arei.gonglei@huawei.com> <5336D059.2080106@redhat.com> <118C69AB-F17E-41B6-AFEF-5C1C02E345C9@icloud.com> In-Reply-To: <118C69AB-F17E-41B6-AFEF-5C1C02E345C9@icloud.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Bvb9fbuugNJX5k77uVJqo43FpAeVGOSrG" Subject: Re: [Qemu-devel] [PATCH] xbzrle: don't check the value in the vm ram repeatedly List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?6ZmI5qKB?= Cc: ChenLiang , weidong.huang@huawei.com, quintela@redhat.com, qemu-devel@nongnu.org, dgilbert@redhat.com, owasserm@redhat.com, arei.gonglei@huawei.com, mrhines@us.ibm.com, pbonzini@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Bvb9fbuugNJX5k77uVJqo43FpAeVGOSrG Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 03/29/2014 08:15 AM, =E9=99=88=E6=A2=81 wrote: >> >> Insufficient. Observe what we did at line 52 when looking for the zer= o-run: >> >>>> /* word at a time for speed */ >>>> if (!res) { >>>> while (i < slen && >>>> (*(long *)(old_buf + i)) =3D=3D (*(long *)(new_buf= + i))) { >> >> This dereferences 8 bytes at new_buf[i] (on a 64-bit machine)... >> >>>> i +=3D sizeof(long); >>>> zrun_len +=3D sizeof(long); >>>> } >>>> >>>> /* go over the rest */ >>>> while (i < slen && old_buf[i] =3D=3D new_buf[i]) { >> >> ...and this also dereferences those same bytes. But your argument is >> that new_buf[i] is volatile. >> >> You MUST read new_buf[i] into a temporary variable, and if the first >> read found a difference, then the "go over the rest" analysis must be = on >> that temporary, rather than going back to reading directly from new_bu= f. >> > It is ok, we just need to guarantee that the pages in cache are same to= the page in dest side.=20 > Don=E2=80=98t care about whether they are same to src side. Because the= modified pages during this > time will be sent at next time. No, it is not okay - if you are reading the same bytes from new_buf, and new_buf is volatile, then you MUST read each byte of new_buf exactly once. Consider this sequence: old_buf contains 00 00 00 00 00 00 00 00 new_buf initially contains 00 00 00 00 00 00 00 01 so the zrun detection spots that the two buffers are different on the 8-byte read. But then another thread modifies new_buf to be all zeros. Now the "go over the rest" loop does 8 reads, expecting to find a difference, but it can't find one. You really need to do the "go over the rest" loop on an 8-byte temporary variable. Ever since your patch made new_buf be a volatile buffer, rather than a static copy, you MUST visit each byte of new_buf exactly on= ce. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Bvb9fbuugNJX5k77uVJqo43FpAeVGOSrG Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTNtf+AAoJEKeha0olJ0NqPi4H/21jXjOVOGLQn6CHI8jEVEFU zcOujnUgebWXns/Yvb34La/TJnMmRegQKigP1pxQ+nBYhW9a7fFVjjIS4uYINFwQ xE9iYUGlcQKuse0kcpr+CJGPj8CoEmPPa3AvKTGvyYTE7fKnbofYIKKGvC0TFeAS RBlZzLUa0/s1Lur6VCSGKkRDUSkVfTrzaGGjisUcp8XIkr1hGal/skiARBzJL+NZ PK5REmJvwl/GqP1BH9jqqta1SQEexaCdGiYGhxT6RL4ADSsPPDT9B3e9s/COzwGw bEdDdRQvHQoZtJw4kNQsx7AAK0sVqgBUVXy6EQ7aHAdLxCw3+YOAYkdZIwtuqn0= =/BCK -----END PGP SIGNATURE----- --Bvb9fbuugNJX5k77uVJqo43FpAeVGOSrG--