Linux Virtualization - Mar 2013 - Opposite return values in vringh_notify_enable_kern() and virtqueue_enable_cb().

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

Sjur Br?ndeland <sjurbren at 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