Re: [PATCH v2] ucc_geth: Reduce IRQ off in xmit path

David Miller [off-list ref] wrote on 2012/09/20 23:08:52:

From: Joakim Tjernlund <redacted>
Date: Thu, 20 Sep 2012 10:17:01 +0200

quoted

Currently ucc_geth_start_xmit wraps IRQ off for the
whole body just to be safe.
Reduce the IRQ off period to a minimum.

Signed-off-by: Joakim Tjernlund <redacted>
---

 v2: Move assignment of ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]]
     inside IRQ off section to prevent racing against
     ucc_geth_tx(). Spotted by Francois Romieu [off-list ref]

I agree with Francois's initial analysis, and disagree with you're
response to him, wrt. the suggest to remove all locking entirely.

Unlike what you claim, there isn't much of a gain at all from merely
make the window of lock holding smaller, especially on the scale
in which you are doing it here.

Whereas removing the lock and the atomic completely, as tg3 does,
will give very significant performance gains.

The locking cost of grabbing the spinlock, and the memory transactions
associated with it, dominate.

Furthermore, even if the gains of your change are non-trivial, you
haven't documented it.  So unless you should some noticable gains from
this, it's just code masterbation as far as I'm concerned and I'm
therefore inclined to not apply patches like this.

TG3's core interrupt locking is not that difficult to understand and
replicate in other drivers, so I dismiss your attempts to avoid that
approach on difficulty grounds as well.

OK, I will give it a go. Got something working that I will send in shortly.
I hope the other patches were OK?

 Jocke