Re: [PATCH 2/3] virtio ring implementation

[Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>]

On Mon, 2007-09-24 at 15:44 +0200, Dor Laor wrote:
> Rusty Russell wrote: 
> > +irqreturn_t vring_interrupt(int irq, void *_vq)
> > +{
> > +       struct vring_virtqueue *vq = to_vvq(_vq);
> > +
> > +       pr_debug("virtqueue interrupt for %p\n", vq);
> > +
> > +       if (unlikely(vq->broken))
> > +               return IRQ_HANDLED;
> > +
> > +       if (more_used(vq)) {
> > +               pr_debug("virtqueue callback for %p (%p)\n",
> > +                        vq, vq->vq.callback);
> > +               if (!vq->vq.callback)
> > +                       return IRQ_NONE;
> > +               if (!vq->vq.callback(&vq->vq))
> > +                       vq->vring.avail->flags |=
> > VRING_AVAIL_F_NO_INTERRUPT;
> > +       } else
> > +               pr_debug("virtqueue %p no more used\n", vq);
> > +
> > +       return IRQ_HANDLED;
> > +}
> > +
> > 
> Seems like there is a problem with shared irq line, the interrupt
> handler always returns IRQ_HANDLED (except for the trivial case
> were the callback is null).

To reply for a second time, this time with thought.  Sorry.

Yes, this code is wrong.  It should be:

irqreturn_t vring_interrupt(int irq, void *_vq)
{
	struct vring_virtqueue *vq = to_vvq(_vq);

	if (!more_used(vq)) {
		pr_debug("virtqueue interrupt with no work for %p\n", vq);
		return IRQ_NONE;
	}

	if (unlikely(vq->broken))
		return IRQ_HANDLED;

	pr_debug("virtqueue callback for %p (%p)\n", vq, vq->vq.callback);
	if (vq->vq.callback && !vq->vq.callback(&vq->vq))
		vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT;

	return IRQ_HANDLED;
}

Cheers,
Rusty.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/