Re: [PATCH 2/5] virtio: support unlocked queue kick

[Rusty Russell]

On Thu, 03 Nov 2011 14:31:48 +1030, Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote:
> So let's change the API for everyone, into:
> 
>         bool virtqueue_should_kick(struct virtqueue *vq);
>         void virtqueue_kick(struct virtqueue *vq);
> 
> Patch series coming...

Nope, that sucked.  virtqueue_should_kick() has side effects (it has to,
if you want the actual kick to be outside the lock).

I stole the names from your patch instead, just added some documentation
and restored the "EXPORT_SYMBOL_GPL(virtqueue_kick);".  No Signed-off-by
on yours.

Also, one trivial nit.  You did:

	bool need_kick = false;
...
	if (vq->event) {
		if (vring_need_event(vring_avail_event(&vq->vring), new, old))
			need_kick = true;
	} else {
		if (!(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY))
			need_kick = true;
	}
...
        return need_kick;

I prefer to use uninitialized vars where possible:

        bool kick;

	if (vq->event) {
		kick = vring_need_event(vring_avail_event(&vq->vring), new, old);
	} else {
		kick = (!(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY));
	}
...
        return kick;

This way, GCC will give an uninitialized var warning if someone changes
the code and forgets to set kick.  This is especially noticeable with
err values and complex functions using goto.

Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/