Manikanta Pubbisetty <mpubbise@xxxxxxxxxxxxxx> writes: > On 6/26/2018 4:55 PM, Toke Høiland-Jørgensen wrote: >> Manikanta Pubbisetty <mpubbise@xxxxxxxxxxxxxx> writes: >> >>> We can still invoke netif stop/wake APIs when wake_tx_queue is >>> implemented but this could lead to packet drops in network layer; >>> adding stop/start logic for software TXQs in mac80211 instead makes >>> more sense; the change proposed adds the same in mac80211. >> I agree with the approach; having packets queued in mac80211 while the >> queues are stopped also means that CoDel can react to them when they are >> resumed again. > > Agreed!! > >> >>> + list_for_each_entry_rcu(sta, &local->sta_list, list) { >>> + if (!atomic_read(&sta->txqs_paused)) >>> + continue; >>> + >>> + atomic_set(&sta->txqs_paused, 0); >> I'm not terribly well-versed in the kernel atomics, but doesn't the >> split of read and set kinda defeat the point of using them? Also, as >> this is under RCU, why do you need an atomic in the first place? > Valid point, txqs_paused is set in ieee80211_tx_dequeue; I was thinking > a case where tx_dequeue is called without rcu_read_lock but seems that > is not the case. All drivers using wake_tx_queue seems are using > rcu_read_lock before tx_dequeue; can there be a case where tx_dequeue is > called without rcu_read_lock? I'm pretty sure we would have other problems if there were... :) -Toke