On Sat, 28 May 2011 23:02:04 +0300, "Michael S. Tsirkin" [off-list ref] wrote:

On Thu, May 26, 2011 at 12:58:23PM +0930, Rusty Russell wrote:

quoted

ie. free two packets for every one we're about to add.  For steady state
that would work really well.

Sure, with indirect buffers, but if we
don't use indirect (and we discussed switching indirect off
dynamically in the past) this becomes harder to
be sure about. I think I understand why but
does not a simple capacity check make it more obvious?

...

quoted

 Then we hit the case where the ring
seems full after we do the add: at that point, screw latency, and just
try to free all the buffers we can.

I see. But the code currently does this:

	for(..)
		get_buf
	add_buf
	if (capacity < max_sk_frags+2) {
		if (!enable_cb)
			for(..)
				get_buf
	}


In other words the second get_buf is only called
in the unlikely case of race condition.

So we'll need to add *another* call to get_buf.
Is it just me or is this becoming messy?

Yes, good point.  I really wonder if anyone would be able to measure the
difference between simply freeing 2 every time (with possible extra
stalls for strange cases) and the more complete version.

But it runs against my grain to implement heuristics when one more call
would make it provably reliable.

Please find a way to make that for loop less ugly though!

Thanks,
Rusty.