Hi Stanislaw, Johannes, On Fri, Mar 9, 2012 at 9:33 AM, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote: > On Fri, 2012-03-09 at 09:22 +0100, Stanislaw Gruszka wrote: > >> > > + 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. OK. Thanks for the explanation. Now it is clear to me. However, I think it would be good to add a comment here, as we are otherwise bound to have the same discussion again in a couple of months time. With that comment added you can add my: Acked-by: Gertjan van Wingerde <gwingerde@xxxxxxxxx> to the v3 patch submission. > > And in fact if you were to use spin_unlock_bh() in that kind of context > it would be a bug :-) > Thanks for your explanation as well, Johannes. Everything is clear to me now :-) -- --- Gertjan -- 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
