[VLAN] spin lock question

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 06 Oct 2006 09:48:27 +0100
Alex Zeffertt <ajz@xxxxxxxxxxxxxxxxxxxxxx> wrote:

> Hi list,
> 
> I've been reading through the 8021q.o module source recently and
> I am confused by the choice of spin lock used in the following
> code:
> 
> 
> int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
>                    struct packet_type* ptype)
> {
> 	...
> 	spin_lock_bh(&vlan_group_lock);
> 	skb->dev = __find_vlan_dev(dev, vid);
> 	if (!skb->dev) {
> 		spin_unlock_bh(&vlan_group_lock);
> 	...
> 
> 
> 
> Now, I may have this wrong but as I understand it spin_lock_bh()
> will disable softIRQs and spin_unlock_bh() will enable softIRQs (*).
> But vlan_skb_recv() is called from within a softIRQ, and so it
> should execute with softIRQs disabled.  Calling spin_unlock_bh()
> will re-enable softIRQs, possibly having undesired consequences.

The _bh is unnecessary and can be removed. It is only called in
RX path and yes softIRQ's are already disabled.  The bh disable/enable
is a counter so it does the right thing when calls are nested like
this. It just wastes a few instructions.



> Should we be using spin_lock_irqsave/spin_lock_irqrestore instead?
> I know that this is more heavy handed in that it stops *all* IRQs,
> but there doesn't seem to be a spin_lock_bhsave/spin_lock_bhrestore!

That is overkill.


-- 
Stephen Hemminger <shemminger@xxxxxxxx>

[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]     [Video 4 Linux]

  Powered by Linux