* Opposite return values in vringh_notify_enable_kern() and virtqueue_enable_cb().
@ 2013-03-11 8:55 Sjur Brændeland
2013-03-12 4:32 ` Rusty Russell
0 siblings, 1 reply; 2+ messages in thread
From: Sjur Brændeland @ 2013-03-11 8:55 UTC (permalink / raw)
To: Rusty Russell; +Cc: Erwan YVIN, virtualization
Hi Rusty,
The two similar functions in vringh and virtqueue for turning on
interrupts has opposite return values if there are buffers available
in the ring. I think it would be better if these two functions aligned
the use of return values. Maybe it's just me, but I got the logic
for re-scheduling NAPI wrong due to this.
/**
* vringh_notify_enable_kern - we want to know if something changes.
* @vrh: the vring.
*
* This always enables notifications, but returns true if there are
* now more buffers available in the vring.
*/
bool vringh_notify_enable_kern(struct vringh *vrh)
/**
* virtqueue_enable_cb - restart callbacks after disable_cb.
* @vq: the struct virtqueue we're talking about.
*
* This re-enables callbacks; it returns "false" if there are pending
* buffers in the queue, ....
*/
bool virtqueue_enable_cb(struct virtqueue *_vq)
Regards.
Sjur
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Opposite return values in vringh_notify_enable_kern() and virtqueue_enable_cb().
2013-03-11 8:55 Opposite return values in vringh_notify_enable_kern() and virtqueue_enable_cb() Sjur Brændeland
@ 2013-03-12 4:32 ` Rusty Russell
0 siblings, 0 replies; 2+ messages in thread
From: Rusty Russell @ 2013-03-12 4:32 UTC (permalink / raw)
To: Sjur Brændeland; +Cc: mst, Erwan YVIN, virtualization
Sjur Brændeland <sjurbren@gmail.com> writes:
> Hi Rusty,
>
> The two similar functions in vringh and virtqueue for turning on
> interrupts has opposite return values if there are buffers available
> in the ring. I think it would be better if these two functions aligned
> the use of return values. Maybe it's just me, but I got the logic
> for re-scheduling NAPI wrong due to this.
Wow. Firstly, the author of the original was an idiot for getting the
API wrong. Secondly, the author of the second was an idiot for making
it different.
So I'm doubly an idiot. If I'd hit that I would have been far less
polite :)
Hmm, wait, vhost has them inverted, so maybe I can blame MST... anyway,
I fixed that too.
Thanks!
Rusty.
> /**
> * vringh_notify_enable_kern - we want to know if something changes.
> * @vrh: the vring.
> *
> * This always enables notifications, but returns true if there are
> * now more buffers available in the vring.
> */
> bool vringh_notify_enable_kern(struct vringh *vrh)
>
> /**
> * virtqueue_enable_cb - restart callbacks after disable_cb.
> * @vq: the struct virtqueue we're talking about.
> *
> * This re-enables callbacks; it returns "false" if there are pending
> * buffers in the queue, ....
> */
> bool virtqueue_enable_cb(struct virtqueue *_vq)
>
> Regards.
> Sjur
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-03-12 4:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-11 8:55 Opposite return values in vringh_notify_enable_kern() and virtqueue_enable_cb() Sjur Brændeland
2013-03-12 4:32 ` Rusty Russell
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.