"Michael S. Tsirkin" [off-list ref] writes:

On Tue, Feb 10, 2015 at 11:33:52AM +1030, Rusty Russell wrote:

quoted

Jason Wang [off-list ref] writes:

quoted

We currently does:

bufs = (avail->idx - last_used_idx) * 3 / 4;

This is ok now since we only try to enable the delayed callbacks when
the queue is about to be full. This may not work well when there is
only one pending buffer in the virtqueue (this may be the case after
tx interrupt was enabled). Since virtqueue_enable_cb() will return
false which may cause unnecessary triggering of napis. This patch
correct this by only calculate the four thirds when bufs is not one.

I mildly prefer to avoid the branch, by changing the calculation like
so:

        /* Set bufs >= 1, even if there's only one pending buffer */
        bufs = (bufs + 1) * 3 / 4;

Or bus * 3/4 + 1

quoted

But it's not clear to me how much this happens.  I'm happy with the
patch though, as currently virtqueue_enable_cb_delayed() is the same
as virtqueue_enable_cb() if there's only been one buffer added.

Cheers,
Rusty.

But isn't this by design?
The documentation says:

 * This re-enables callbacks but hints to the other side to delay
 * interrupts until most of the available buffers have been processed;

Clearly, this implies that when there's one buffer it must behave
exactly the same.

Yes, my mistake.  We could hit the "never gets notified" case with this
change, and that's a much bigger problem.

So I don't think we can accept this patch...
Rusty.