* [Qemu-devel] [PATCH 0/2] xenfb bug fix and clean up
@ 2016-04-12 9:57 Wei Liu
2016-04-12 9:57 ` [Qemu-devel] [PATCH 1/2] xenfb: use the correct condition to avoid excessive looping Wei Liu
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Wei Liu @ 2016-04-12 9:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Xen-devel, Wei Liu
Wei Liu (2):
xenfb: use the correct condition to avoid excessive looping
xenfb: remove out_cons in xenfb_handle_events
hw/display/xenfb.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 1/2] xenfb: use the correct condition to avoid excessive looping 2016-04-12 9:57 [Qemu-devel] [PATCH 0/2] xenfb bug fix and clean up Wei Liu @ 2016-04-12 9:57 ` Wei Liu 2016-04-12 10:04 ` Anthony PERARD 2016-04-12 10:04 ` Anthony PERARD 2016-04-12 9:57 ` Wei Liu 2016-04-12 9:57 ` Wei Liu 2 siblings, 2 replies; 11+ messages in thread From: Wei Liu @ 2016-04-12 9:57 UTC (permalink / raw) To: qemu-devel; +Cc: Xen-devel, Wei Liu, Stefano Stabellini, Anthony Perard In commit ac0487e1 ("xenfb.c: avoid expensive loops when prod <= out_cons"), ">=" was used. In fact, a full ring is a legit state. Correct the test to use ">". Reported-by: "Hao, Xudong" <xudong.hao@intel.com> Signed-off-by: Wei Liu <wei.liu2@citrix.com> Tested-by: "Hao, Xudong" <xudong.hao@intel.com> --- Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Anthony Perard <anthony.perard@citrix.com> Backport candidate to our own tree. --- hw/display/xenfb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index 40b096a..9866dfd 100644 --- a/hw/display/xenfb.c +++ b/hw/display/xenfb.c @@ -775,7 +775,7 @@ static void xenfb_handle_events(struct XenFB *xenfb) prod = page->out_prod; out_cons = page->out_cons; - if (prod - out_cons >= XENFB_OUT_RING_LEN) { + if (prod - out_cons > XENFB_OUT_RING_LEN) { return; } xen_rmb(); /* ensure we see ring contents up to prod */ -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] xenfb: use the correct condition to avoid excessive looping 2016-04-12 9:57 ` [Qemu-devel] [PATCH 1/2] xenfb: use the correct condition to avoid excessive looping Wei Liu @ 2016-04-12 10:04 ` Anthony PERARD 2016-04-12 10:04 ` Anthony PERARD 1 sibling, 0 replies; 11+ messages in thread From: Anthony PERARD @ 2016-04-12 10:04 UTC (permalink / raw) To: Wei Liu; +Cc: qemu-devel, Xen-devel, Stefano Stabellini On Tue, Apr 12, 2016 at 10:57:16AM +0100, Wei Liu wrote: > In commit ac0487e1 ("xenfb.c: avoid expensive loops when prod <= > out_cons"), ">=" was used. In fact, a full ring is a legit state. > Correct the test to use ">". > > Reported-by: "Hao, Xudong" <xudong.hao@intel.com> > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > Tested-by: "Hao, Xudong" <xudong.hao@intel.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com> > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Anthony Perard <anthony.perard@citrix.com> > > Backport candidate to our own tree. > --- > hw/display/xenfb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c > index 40b096a..9866dfd 100644 > --- a/hw/display/xenfb.c > +++ b/hw/display/xenfb.c > @@ -775,7 +775,7 @@ static void xenfb_handle_events(struct XenFB *xenfb) > > prod = page->out_prod; > out_cons = page->out_cons; > - if (prod - out_cons >= XENFB_OUT_RING_LEN) { > + if (prod - out_cons > XENFB_OUT_RING_LEN) { > return; > } > xen_rmb(); /* ensure we see ring contents up to prod */ > -- > 2.1.4 > -- Anthony PERARD ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] xenfb: use the correct condition to avoid excessive looping 2016-04-12 9:57 ` [Qemu-devel] [PATCH 1/2] xenfb: use the correct condition to avoid excessive looping Wei Liu 2016-04-12 10:04 ` Anthony PERARD @ 2016-04-12 10:04 ` Anthony PERARD 1 sibling, 0 replies; 11+ messages in thread From: Anthony PERARD @ 2016-04-12 10:04 UTC (permalink / raw) To: Wei Liu; +Cc: Xen-devel, Stefano Stabellini, qemu-devel On Tue, Apr 12, 2016 at 10:57:16AM +0100, Wei Liu wrote: > In commit ac0487e1 ("xenfb.c: avoid expensive loops when prod <= > out_cons"), ">=" was used. In fact, a full ring is a legit state. > Correct the test to use ">". > > Reported-by: "Hao, Xudong" <xudong.hao@intel.com> > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > Tested-by: "Hao, Xudong" <xudong.hao@intel.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com> > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Anthony Perard <anthony.perard@citrix.com> > > Backport candidate to our own tree. > --- > hw/display/xenfb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c > index 40b096a..9866dfd 100644 > --- a/hw/display/xenfb.c > +++ b/hw/display/xenfb.c > @@ -775,7 +775,7 @@ static void xenfb_handle_events(struct XenFB *xenfb) > > prod = page->out_prod; > out_cons = page->out_cons; > - if (prod - out_cons >= XENFB_OUT_RING_LEN) { > + if (prod - out_cons > XENFB_OUT_RING_LEN) { > return; > } > xen_rmb(); /* ensure we see ring contents up to prod */ > -- > 2.1.4 > -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] xenfb: use the correct condition to avoid excessive looping 2016-04-12 9:57 [Qemu-devel] [PATCH 0/2] xenfb bug fix and clean up Wei Liu 2016-04-12 9:57 ` [Qemu-devel] [PATCH 1/2] xenfb: use the correct condition to avoid excessive looping Wei Liu @ 2016-04-12 9:57 ` Wei Liu 2016-04-12 9:57 ` Wei Liu 2 siblings, 0 replies; 11+ messages in thread From: Wei Liu @ 2016-04-12 9:57 UTC (permalink / raw) To: qemu-devel; +Cc: Anthony Perard, Xen-devel, Stefano Stabellini, Wei Liu In commit ac0487e1 ("xenfb.c: avoid expensive loops when prod <= out_cons"), ">=" was used. In fact, a full ring is a legit state. Correct the test to use ">". Reported-by: "Hao, Xudong" <xudong.hao@intel.com> Signed-off-by: Wei Liu <wei.liu2@citrix.com> Tested-by: "Hao, Xudong" <xudong.hao@intel.com> --- Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Anthony Perard <anthony.perard@citrix.com> Backport candidate to our own tree. --- hw/display/xenfb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index 40b096a..9866dfd 100644 --- a/hw/display/xenfb.c +++ b/hw/display/xenfb.c @@ -775,7 +775,7 @@ static void xenfb_handle_events(struct XenFB *xenfb) prod = page->out_prod; out_cons = page->out_cons; - if (prod - out_cons >= XENFB_OUT_RING_LEN) { + if (prod - out_cons > XENFB_OUT_RING_LEN) { return; } xen_rmb(); /* ensure we see ring contents up to prod */ -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 2/2] xenfb: remove out_cons in xenfb_handle_events 2016-04-12 9:57 [Qemu-devel] [PATCH 0/2] xenfb bug fix and clean up Wei Liu @ 2016-04-12 9:57 ` Wei Liu 2016-04-12 9:57 ` Wei Liu 2016-04-12 9:57 ` Wei Liu 2 siblings, 0 replies; 11+ messages in thread From: Wei Liu @ 2016-04-12 9:57 UTC (permalink / raw) To: qemu-devel; +Cc: Xen-devel, Wei Liu, Stefano Stabellini, Anthony Perard 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; + 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; -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] xenfb: remove out_cons in xenfb_handle_events @ 2016-04-12 9:57 ` Wei Liu 0 siblings, 0 replies; 11+ messages in thread From: Wei Liu @ 2016-04-12 9:57 UTC (permalink / raw) To: qemu-devel; +Cc: Anthony Perard, Xen-devel, Stefano Stabellini, Wei Liu 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; + 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; -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xenfb: remove out_cons in xenfb_handle_events 2016-04-12 9:57 ` Wei Liu @ 2016-04-12 9:59 ` Andrew Cooper -1 siblings, 0 replies; 11+ messages in thread From: Andrew Cooper @ 2016-04-12 9:59 UTC (permalink / raw) To: Wei Liu, qemu-devel; +Cc: Anthony Perard, Xen-devel, Stefano Stabellini 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; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xenfb: remove out_cons in xenfb_handle_events @ 2016-04-12 9:59 ` Andrew Cooper 0 siblings, 0 replies; 11+ messages in thread From: Andrew Cooper @ 2016-04-12 9:59 UTC (permalink / raw) To: Wei Liu, qemu-devel; +Cc: Anthony Perard, Xen-devel, Stefano Stabellini 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; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xenfb: remove out_cons in xenfb_handle_events 2016-04-12 9:59 ` Andrew Cooper (?) @ 2016-04-12 10:04 ` Wei Liu -1 siblings, 0 replies; 11+ messages in thread From: Wei Liu @ 2016-04-12 10:04 UTC (permalink / raw) To: Andrew Cooper Cc: Wei Liu, qemu-devel, 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 <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; > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xenfb: remove out_cons in xenfb_handle_events 2016-04-12 9:59 ` Andrew Cooper (?) (?) @ 2016-04-12 10:04 ` Wei Liu -1 siblings, 0 replies; 11+ messages in thread From: Wei Liu @ 2016-04-12 10:04 UTC (permalink / raw) To: Andrew Cooper Cc: Anthony Perard, Xen-devel, Stefano Stabellini, Wei Liu, qemu-devel 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; > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-04-12 10:04 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-12 9:57 [Qemu-devel] [PATCH 0/2] xenfb bug fix and clean up Wei Liu 2016-04-12 9:57 ` [Qemu-devel] [PATCH 1/2] xenfb: use the correct condition to avoid excessive looping Wei Liu 2016-04-12 10:04 ` Anthony PERARD 2016-04-12 10:04 ` Anthony PERARD 2016-04-12 9:57 ` Wei Liu 2016-04-12 9:57 ` [Qemu-devel] [PATCH 2/2] xenfb: remove out_cons in xenfb_handle_events Wei Liu 2016-04-12 9:57 ` Wei Liu 2016-04-12 9:59 ` [Qemu-devel] [Xen-devel] " Andrew Cooper 2016-04-12 9:59 ` Andrew Cooper 2016-04-12 10:04 ` [Qemu-devel] [Xen-devel] " Wei Liu 2016-04-12 10:04 ` Wei Liu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.