From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Lj8IX-0000Nd-EZ for qemu-devel@nongnu.org; Mon, 16 Mar 2009 04:35:49 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Lj8IR-0000I7-Cy for qemu-devel@nongnu.org; Mon, 16 Mar 2009 04:35:48 -0400 Received: from [199.232.76.173] (port=46952 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Lj8IR-0000Hi-2Z for qemu-devel@nongnu.org; Mon, 16 Mar 2009 04:35:43 -0400 Received: from mx2.redhat.com ([66.187.237.31]:59430) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Lj8IQ-0001Uw-I1 for qemu-devel@nongnu.org; Mon, 16 Mar 2009 04:35:42 -0400 Message-ID: <49BE0F50.5000009@redhat.com> Date: Mon, 16 Mar 2009 09:35:28 +0100 From: Gerd Hoffmann MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] vnc: shared buffer: skip some optimizations. References: <49B7CE21.4030104@redhat.com> <49B7DC25.6010500@codemonkey.ws> <49B9697C.5020607@redhat.com> <49BA4B9B.7040208@eu.citrix.com> In-Reply-To: <49BA4B9B.7040208@eu.citrix.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: "qemu-devel@nongnu.org" Stefano Stabellini wrote: > Gerd Hoffmann wrote: > >> Sending screen updates to the client will be done using the server >> surface + dirty map only, so the guest updating the screen in parallel >> can't cause trouble here. > > Could you please explain an example of trouble caused by sich situations? Sure. > Given A, B and C three different points in time with A < B < C, > currently what we are doing is: > > A) getting the guest dirty map > > B) filtering the guest dirty map using memcmp with the server surface > (called old_data in the current code), however the dirty map could be > inconsistent with the framebuffer because was set in A > > C) sending updates to the client from the guest surface based on the > dirty map, however the dirty map is inconsistent because it was set in A > and filtered in B The dirty bitmap inconsistency isn't a problem at all. The next update round will fix that. The trouble spot is here: The guest might have updated the screen between (b) and (c). This will cause the vnc clients view of the screen content become inconsistent with old_data. On the next update round (b) will stop working correctly due to that. What my patch does in terms of the old code is make step (c) use old_data instead of the guest-visible framebuffer. I didn't like stacking hacks on hacks like this though, so I decided to explicitly name the two surfaces. You still could get away with just one dirty bitmap for both surfaces. Having two bitmaps IMHO makes the code more readable though. It also allows killing the memset(old_data,42,size) hack for forced client updates (see framebuffer_update_request()). An -- in preparation for the future -- it makes the vnc code a bit more thread-friendly. Guest screen updating (aka vnc_update()) can run in parallel with encoding updates for the guest (i.e. step (c)). > If your goal is to send the most updated version of the framebuffer to > the guest ASAP, the current code is better at it. But *that* is exactly where the bug is ;) cheers, Gerd