"Michael S. Tsirkin" <mst@xxxxxxxxxx> writes: > On Tue, Feb 10, 2015 at 11:33:52AM +1030, Rusty Russell wrote: >> Jason Wang <jasowang@xxxxxxxxxx> writes: >> > 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 > >> 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. _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
