| Message ID | 1460455037-28213-3-git-send-email-wei.liu2@citrix.com |
|---|---|
| State | New |
| Headers | show |
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 <wei.liu2@citrix.com> > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Anthony Perard <anthony.perard@citrix.com> > --- > 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. ~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;
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 <wei.liu2@citrix.com> > > --- > > Cc: Stefano Stabellini <sstabellini@kernel.org> > > Cc: Anthony Perard <anthony.perard@citrix.com> > > --- > > 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; >
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; + 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;
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 <wei.liu2@citrix.com> --- Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Anthony Perard <anthony.perard@citrix.com> --- hw/display/xenfb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)