| Message ID | 20240819065855.25678-1-pkshih@realtek.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] wifi: mac80211: export ieee80211_purge_tx_queue() for drivers | expand |
Johannes Berg <johannes@sipsolutions.net> wrote: > > +/** > > + * ieee80211_purge_tx_queue - purge TX skb queue > > + * @hw: the hardware > > + * @skbs: the skbs > > + * > > + * Free a set of transmit skbs. Use this function when device is going to stop > > + * but some transmit skbs without TX status are still queued. > > + */ > > +void ieee80211_purge_tx_queue(struct ieee80211_hw *hw, > > + struct sk_buff_head *skbs); > > Unlike skb_queue_purge()/skb_queue_purge_reason(), this doesn't take the > lock, that seems important to note here? Thanks. I wasn't aware of lock, and rtw88 needs a lock version because its TX work isn't stopped yet while calling ieee80211_purge_tx_queue(). I have three candidate options: 1. add spin_lock by driver before calling ieee80211_purge_tx_queue(). 2. move original ieee80211_purge_tx_queue() to __ieee80211_purge_tx_queue(), and add an new lock version of ieee80211_purge_tx_queue(). 3. change rtw88 deinit flow to ensure stopping TX work before calling ieee80211_purge_tx_queue() I prefer option 3. But want your opinion if option 2 is worth to do? Ping-Ke
Ping-Ke Shih <pkshih@realtek.com> wrote: > Johannes Berg <johannes@sipsolutions.net> wrote: > > > +/** > > > + * ieee80211_purge_tx_queue - purge TX skb queue > > > + * @hw: the hardware > > > + * @skbs: the skbs > > > + * > > > + * Free a set of transmit skbs. Use this function when device is going to stop > > > + * but some transmit skbs without TX status are still queued. > > > + */ > > > +void ieee80211_purge_tx_queue(struct ieee80211_hw *hw, > > > + struct sk_buff_head *skbs); > > > > Unlike skb_queue_purge()/skb_queue_purge_reason(), this doesn't take the > > lock, that seems important to note here? > > Thanks. I wasn't aware of lock, and rtw88 needs a lock version because > its TX work isn't stopped yet while calling ieee80211_purge_tx_queue(). > > I have three candidate options: > > 1. add spin_lock by driver before calling ieee80211_purge_tx_queue(). > > 2. move original ieee80211_purge_tx_queue() to __ieee80211_purge_tx_queue(), > and add an new lock version of ieee80211_purge_tx_queue(). > > 3. change rtw88 deinit flow to ensure stopping TX work before calling > ieee80211_purge_tx_queue() > > I prefer option 3. But want your opinion if option 2 is worth to do? > I have chosen option 3, so just export ieee80211_purge_tx_queue() and no need to touch ieee80211_purge_tx_queue() further. Sent v2 along with suggested comments.
diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 0a04eaf5343c..f11844f0c80f 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -3180,6 +3180,17 @@ ieee80211_get_alt_retry_rate(const struct ieee80211_hw *hw, */ void ieee80211_free_txskb(struct ieee80211_hw *hw, struct sk_buff *skb); +/** + * ieee80211_purge_tx_queue - purge TX skb queue + * @hw: the hardware + * @skbs: the skbs + * + * Free a set of transmit skbs. Use this function when device is going to stop + * but some transmit skbs without TX status are still queued. + */ +void ieee80211_purge_tx_queue(struct ieee80211_hw *hw, + struct sk_buff_head *skbs); + /** * DOC: Hardware crypto acceleration * diff --git a/net/mac80211/status.c b/net/mac80211/status.c index dd8f857a1fbc..d1cf987de13b 100644 --- a/net/mac80211/status.c +++ b/net/mac80211/status.c @@ -1301,3 +1301,4 @@ void ieee80211_purge_tx_queue(struct ieee80211_hw *hw, while ((skb = __skb_dequeue(skbs))) ieee80211_free_txskb(hw, skb); } +EXPORT_SYMBOL(ieee80211_purge_tx_queue);
Drivers need to purge TX SKB when stopping. Use skb_queue_purge() can't report TX status to mac80211, causing ieee80211_free_ack_frame() warns "Have pending ack frames!". Signed-off-by: Ping-Ke Shih <pkshih@realtek.com> --- Hi Johannes, If this patch is accepted, please merge it ahead and I will merge the second patch via rtw tree after pulling wireless-next tree. I think this can reduce possible conflicts. --- include/net/mac80211.h | 11 +++++++++++ net/mac80211/status.c | 1 + 2 files changed, 12 insertions(+)