Re: [PATCH net V2] xen-netback: don't move event pointer in TX credit timeout callback

On Thu, May 15, 2014 at 05:34:09PM +0100, Zoltan Kiss wrote:
[...]

quoted

quoted

quoted

 		RING_FINAL_CHECK_FOR_REQUESTS(&vif->tx, more_to_do);
-		if (!more_to_do)
+		if (!more_to_do || rate_limited)

How about calling timer_pending(&vif->credit_timeout) instead?

timer_pending(&vif->credit_timeout) covers only one of two senarios of
"credit exceeded", see tx_credit_exceeded.

The other scenario is when the packet size exceeds the credit. There is no
packet here actually, we just want to know if this vif ran out of credit and
waiting for the timer to fire.

quoted

Which place are you referring to? There's packet in the ring, right? So
you're saying in xenvif_poll "more_to_do" is true and "timer_pending" is
also true when we come to xenvif_poll again?

quoted

quoted

Also, can this __napi_complete and the callback's napi_schedule race with
each other? When napi_complete is between removing from the list and
clearing the bit, and napi_schedule is just test&set the bit, the latter
won't add the instance to the list again

I think it should be fine. How is it different from what we already have
now? Is this something similar to what David once posted?

  [ref]

Unfortunately that discussion stalled, and my question were not answered, so
I bumped it again. But that's different a bit: it was about racing between
the NAPI instance (running in softirq context) and the interrupt. Here the
danger is that the NAPI instance and the softirq can race. They both run in
softirq context, and even if they were originally on the same CPU, I'm sure
if the instance move somewhere else, the timer doesn't follow it.

This comes back to that original question, doesn't it? That's NAPI
running on CPU A and raised by CPU B.

Wei.

Zoli