From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50267) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1apvAW-0002uy-Vs for qemu-devel@nongnu.org; Tue, 12 Apr 2016 06:03:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1apvAS-0005y2-5V for qemu-devel@nongnu.org; Tue, 12 Apr 2016 06:03:36 -0400 Received: from smtp.citrix.com ([66.165.176.89]:21506) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1apvAS-0005xr-20 for qemu-devel@nongnu.org; Tue, 12 Apr 2016 06:03:32 -0400 Date: Tue, 12 Apr 2016 11:04:58 +0100 From: Wei Liu Message-ID: <20160412100458.GB18652@citrix.com> References: <1460455037-28213-1-git-send-email-wei.liu2@citrix.com> <1460455037-28213-3-git-send-email-wei.liu2@citrix.com> <570CC719.9000800@citrix.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <570CC719.9000800@citrix.com> Subject: Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xenfb: remove out_cons in xenfb_handle_events List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrew Cooper Cc: Wei Liu , qemu-devel@nongnu.org, Anthony Perard , Xen-devel , Stefano Stabellini On Tue, Apr 12, 2016 at 10:59:53AM +0100, Andrew Cooper wrote: > On 12/04/16 10:57, Wei Liu wrote: > > The variable out_cons was only used to temporarily hold the consumer > > index. Use cons directly to simplify code a bit. > > > > No functional change introduced. > > > > Signed-off-by: Wei Liu > > --- > > Cc: Stefano Stabellini > > Cc: Anthony Perard > > --- > > hw/display/xenfb.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c > > index 9866dfd..78a8dcd 100644 > > --- a/hw/display/xenfb.c > > +++ b/hw/display/xenfb.c > > @@ -770,16 +770,16 @@ static void xenfb_invalidate(void *opaque) > > > > static void xenfb_handle_events(struct XenFB *xenfb) > > { > > - uint32_t prod, cons, out_cons; > > + uint32_t prod, cons; > > struct xenfb_page *page = xenfb->c.page; > > > > prod = page->out_prod; > > - out_cons = page->out_cons; > > - if (prod - out_cons > XENFB_OUT_RING_LEN) { > > + cons = page->out_cons; > > You need the xen_rmb() before the first use of prod or cons. > Good point. I overlooked that. The change should belong to either previous patch or a new patch though. Wei. > ~Andrew > > > + if (prod - cons > XENFB_OUT_RING_LEN) { > > return; > > } > > xen_rmb(); /* ensure we see ring contents up to prod */ > > - for (cons = out_cons; cons != prod; cons++) { > > + for ( ; cons != prod; cons++) { > > union xenfb_out_event *event = &XENFB_OUT_RING_REF(page, cons); > > uint8_t type = event->type; > > int x, y, w, h; >