On 23 January 2012 11:27, Helmut Schaa <helmut.sc...@googlemail.com> wrote:
> 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 ...

I will test more thoroughly but I noticed about 0.7 MB/s speed drop
comparing with those spinlocks commented out.

>>> 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.

Yes, but houskeeping tasklet is also called from other places too.
And as I understand interrupts are disabled before calling tasklet anyway?
I can't say much about what is right and what is wrong but reading
about tasklets 
(http://home.comcast.net/~heidi.young1/projects/cs517b/linuxkernel_ch6/tasklets.htm)
I noticed that before scheduling a tasklet irq state have to be saved
(I suppose that is what irqsave is for)?
Sorry for the dumb questions if they are.


Regards,
Roman
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to