diff mbox

pv-grub guest booting fail with recent qemu-xen

Message ID 20160411105406.GC1346@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wei Liu April 11, 2016, 10:54 a.m. UTC 
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?


> Samuel

Comments

Wei Liu April 11, 2016, 11:52 a.m. UTC | #1 
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...
Hao, Xudong April 12, 2016, 1:56 a.m. UTC | #2 
> -----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
Wei Liu April 12, 2016, 8:28 a.m. UTC | #3 
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 mbox

Patch

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 */