"Michael S. Tsirkin" <m...@redhat.com> writes: > On Tue, Feb 10, 2015 at 11:33:52AM +1030, Rusty Russell wrote: >> Jason Wang <jasow...@redhat.com> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/