On Sun, Jan 22, 2012 at 11:44 AM, John Crispin <j...@phrozen.org> wrote:
> On 22/01/12 02:45, Roman Yeryomin wrote:
>> On 17 January 2012 11:42, Helmut Schaa <helmut.sc...@googlemail.com> wrote:
>>> @@ -313,6 +312,7 @@ ramips_eth_tx_housekeeping(unsigned long ptr)
>>> struct net_device *dev = (struct net_device*)ptr;
>>> struct raeth_priv *priv = netdev_priv(dev);
>>>
>>> + spin_lock(&priv->page_lock);
>>> while ((priv->tx[priv->skb_free_idx].txd2 & TX_DMA_DONE) &&
>>> (priv->tx_skb[priv->skb_free_idx])) {
>>> dev_kfree_skb_irq(priv->tx_skb[priv->skb_free_idx]);
>>> @@ -321,6 +321,7 @@ ramips_eth_tx_housekeeping(unsigned long ptr)
>>> if (priv->skb_free_idx >= NUM_TX_DESC)
>>> priv->skb_free_idx = 0;
>>> }
>>> + spin_unlock(&priv->page_lock);
>>>
>>> ramips_fe_int_enable(RAMIPS_TX_DLY_INT);
>>> }
>>
>> Seems that spinlock here introduces performance hit in wifi<->eth path.
You're sure? I can still get 60Mbit/s TCP bridged between eth <->wifi and
on UP (like all ramips devices) systems the spinlock should be optimized
out anyway. The only overhead that gets added is that housekeeping is
done in softirq context instead of hardirq but that is justified as we should
only do _minimal_ work in the actual IRQ handler ...
>> Are you sure it's needed?
No, but the spinlock_irqsave locked between the housekeeping path
and the tx path. The new locking just does the same without the need to
disabled IRQs.
> Seems that it works fine also without this spinlock.
I wouldn't remove the spinlock without very good code review! If you
can explain why it is not needed I'm fine with removing it but I haven't
done an exhausting review yet, so this might take some time ...
Helmut
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
