Hi Gertjan On Thu, Mar 08, 2012 at 10:40:43PM +0100, Gertjan van Wingerde wrote: > > @@ -152,13 +152,20 @@ void rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb) > > if (unlikely(rt2x00queue_write_tx_frame(queue, skb, false))) > > goto exit_fail; > > > > + /* > > + * Pausing queue has to be serialized with rt2x00lib_txdone() . > > + */ > > + spin_lock(&queue->tx_lock); > > if (rt2x00queue_threshold(queue)) > > rt2x00queue_pause_queue(queue); > > + spin_unlock(&queue->tx_lock); > > > > return; > > > > exit_fail: > > + spin_lock(&queue->tx_lock); > > rt2x00queue_pause_queue(queue); > > + spin_unlock(&queue->tx_lock); > > exit_free_skb: > > ieee80211_free_txskb(hw, skb); > > } > > I'm sorry, but I'm still not convinced that we can use spin_lock_bh in > one place of the code and then spin_lock in another place of the code, > using the *same* spinlock. > I always use the cheat sheet shown in: > http://www.kernel.org/pub/linux/kernel/people/rusty/kernel-locking/c214.html > > which to me shows that by definition we should be using spin_lock_bh in > all cases now, the new ones and the existing cases where we lock tx_lock. We have bh disabled here since ieee80211_xmit is always called with bh disabled (early on dev_queue_xmit() or in ieee80211_tx_skb_tid()). I can add comment about that. Additionally I ran patch with CONFIG_LOCKDEP which is capable to detect locking context errors and no warning was printed. Thanks Stanislaw -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
