Hi, Ron, I noticed you added spinlocks to ampdu_mlme for both the and TX processing. Generally, the locking in sta_info is lacking, for example the flags member is simply updated all over the place without regard for locking. I have boiled down the locking requirements to various things: * static sta info members, set on allocation (fine now after a patch that splits allocation and hash table insertion, except in one place where an AP entry is added, need to review) * semi-static information, to review, but most likely updated only on external events like cfg80211 which are synchronized * flags, needs locking badly * powersave frame queues (have internal locking) * sta info members updated only within the TX status path -> fine per my previous mail * sta info members updated only within the RX path -> also fine per my previous mail * debug counters we don't really care about (channel use) * AMPDU stuff (own locking) Hence, I think we can actually get away without more locking if we protect the flags better. Should we use a spinlock or the atomic set_bit()/clear_bit()/etc. operations? If we were to use a spinlock, could the AMPDU stuff use that too? Does it really need one spinlock for TX and one for RX? I don't think the spinlock would be contended a lot, but it is possible that, at the same time, * the TX path is trying to clear the PSPOLL flag * hostapd is trying to clear the WLAN_STA_AUTHORIZED flag * the RX path is trying to set the PS flag .... I hope you're all (not just Ron, I just addressed you because of the ampdu stuff) reading this and trying to wrap your head around the synchronisation issues in mac80211, I don't want to be the only one with then somehow magic knowledge about it... Once I understand it I'll try to document it too. johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
