Message ID | 20160411105406.GC1346@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Mon, Apr 11, 2016 at 11:54:06AM +0100, Wei Liu wrote: > On Sun, Apr 10, 2016 at 10:14:46PM +0200, Samuel Thibault wrote: > > Hello, > > > > > > > > + if (prod - out_cons >= XENFB_OUT_RING_LEN) { > > > > > > + return; > > > > > > + } > > > > This test seems overzealous to me: AIUI, the producer can produce > > XENFB_OUT_RING_LEN events, and thus prod - out_cons is exactly > > XENFB_OUT_RING_LEN, i.e. there is no room left at all. > > > > The frontend part is: > > > > while (page->out_prod - page->out_cons == XENFB_OUT_RING_LEN) > > schedule(); > > > > I.e. it waits while the buffer is exactly full. > > > > So it seems to me the bug is at the backend side. > > > > OK, thanks for checking! > > Hudong, can you try the following diff? ^ Xudong Sorry for the typo...
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@citrix.com] > Sent: Monday, April 11, 2016 6:54 PM > To: Samuel Thibault <samuel.thibault@ens-lyon.org>; Wei Liu > <wei.liu2@citrix.com>; Hao, Xudong <xudong.hao@intel.com>; Konrad > Rzeszutek Wilk <konrad.wilk@oracle.com>; xen-devel@lists.xen.org; > stefano.stabellini@eu.citrix.com; Anthony PERARD <anthony.perard@citrix.com> > Subject: Re: [Xen-devel] pv-grub guest booting fail with recent qemu-xen > > On Sun, Apr 10, 2016 at 10:14:46PM +0200, Samuel Thibault wrote: > > Hello, > > > > > > > > + if (prod - out_cons >= XENFB_OUT_RING_LEN) { > > > > > > + return; > > > > > > + } > > > > This test seems overzealous to me: AIUI, the producer can produce > > XENFB_OUT_RING_LEN events, and thus prod - out_cons is exactly > > XENFB_OUT_RING_LEN, i.e. there is no room left at all. > > > > The frontend part is: > > > > while (page->out_prod - page->out_cons == XENFB_OUT_RING_LEN) > > schedule(); > > > > I.e. it waits while the buffer is exactly full. > > > > So it seems to me the bug is at the backend side. > > > > OK, thanks for checking! > > Hudong, can you try the following diff? Yes, This patch works with testing. Thanks, -Xudong > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index 8eb3046..284b345 > 100644 > --- a/hw/display/xenfb.c > +++ b/hw/display/xenfb.c > @@ -789,7 +789,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 */ > > > Samuel
On Tue, Apr 12, 2016 at 01:56:50AM +0000, Hao, Xudong wrote: > > -----Original Message----- > > From: Wei Liu [mailto:wei.liu2@citrix.com] > > Sent: Monday, April 11, 2016 6:54 PM > > To: Samuel Thibault <samuel.thibault@ens-lyon.org>; Wei Liu > > <wei.liu2@citrix.com>; Hao, Xudong <xudong.hao@intel.com>; Konrad > > Rzeszutek Wilk <konrad.wilk@oracle.com>; xen-devel@lists.xen.org; > > stefano.stabellini@eu.citrix.com; Anthony PERARD <anthony.perard@citrix.com> > > Subject: Re: [Xen-devel] pv-grub guest booting fail with recent qemu-xen > > > > On Sun, Apr 10, 2016 at 10:14:46PM +0200, Samuel Thibault wrote: > > > Hello, > > > > > > > > > > + if (prod - out_cons >= XENFB_OUT_RING_LEN) { > > > > > > > + return; > > > > > > > + } > > > > > > This test seems overzealous to me: AIUI, the producer can produce > > > XENFB_OUT_RING_LEN events, and thus prod - out_cons is exactly > > > XENFB_OUT_RING_LEN, i.e. there is no room left at all. > > > > > > The frontend part is: > > > > > > while (page->out_prod - page->out_cons == XENFB_OUT_RING_LEN) > > > schedule(); > > > > > > I.e. it waits while the buffer is exactly full. > > > > > > So it seems to me the bug is at the backend side. > > > > > > > OK, thanks for checking! > > > > Hudong, can you try the following diff? > > Yes, This patch works with testing. > Thanks for testing! I will submit this patch properly. Wei.
diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index 8eb3046..284b345 100644 --- a/hw/display/xenfb.c +++ b/hw/display/xenfb.c @@ -789,7 +789,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 */