> From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
>
> The old code might not ever run the stale skb status processing
> list, so change it to ensure the stale entries are cleaned up
> regularly.
I guess this work is already performed in mt76_tx_status_check() executed by
mac work (e.g. mt7921_mac_work()) where pid is set to 0 and the first lookup
will always fail. Have you identified an issue in the current codebase?
>
> Signed-off-by: Ben Greear <greearb@xxxxxxxxxxxxxxx>
> ---
> drivers/net/wireless/mediatek/mt76/mt76.h | 1 +
> drivers/net/wireless/mediatek/mt76/tx.c | 24 +++++++++++++++++------
> 2 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 37d82d806c09..bfb68788251f 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -271,6 +271,7 @@ struct mt76_wcid {
>
> struct list_head list;
> struct idr pktid;
> + unsigned long last_idr_check_at; /* in jiffies */
> };
>
> struct mt76_txq {
> diff --git a/drivers/net/wireless/mediatek/mt76/tx.c b/drivers/net/wireless/mediatek/mt76/tx.c
> index 938353ac272f..b6f0d74fd563 100644
> --- a/drivers/net/wireless/mediatek/mt76/tx.c
> +++ b/drivers/net/wireless/mediatek/mt76/tx.c
> @@ -157,24 +157,35 @@ mt76_tx_status_skb_get(struct mt76_dev *dev, struct mt76_wcid *wcid, int pktid,
> struct sk_buff_head *list)
> {
> struct sk_buff *skb;
> + struct sk_buff *skb2;
> int id;
> + /* Check twice as often as the timeout value so that we mitigate
> + * worse-case timeout detection (where we do the check right before
> + * the per skb timer would have expired and so have to wait another interval
> + * to detect the skb status timeout.)
> + */
> + static const int check_interval = MT_TX_STATUS_SKB_TIMEOUT / 2;
>
> lockdep_assert_held(&dev->status_lock);
>
> skb = idr_remove(&wcid->pktid, pktid);
> - if (skb)
> +
> + /* If we have not checked for stale entries recently,
> + * then do that check now.
> + */
> + if (time_is_after_jiffies(wcid->last_idr_check_at + check_interval))
> goto out;
>
> /* look for stale entries in the wcid idr queue */
> - idr_for_each_entry(&wcid->pktid, skb, id) {
> - struct mt76_tx_cb *cb = mt76_tx_skb_cb(skb);
> + idr_for_each_entry(&wcid->pktid, skb2, id) {
> + struct mt76_tx_cb *cb = mt76_tx_skb_cb(skb2);
>
> if (pktid >= 0) {
> if (!(cb->flags & MT_TX_CB_DMA_DONE))
> continue;
>
> if (time_is_after_jiffies(cb->jiffies +
> - MT_TX_STATUS_SKB_TIMEOUT))
> + MT_TX_STATUS_SKB_TIMEOUT))
> continue;
> }
>
> @@ -182,9 +193,10 @@ mt76_tx_status_skb_get(struct mt76_dev *dev, struct mt76_wcid *wcid, int pktid,
> * and stop waiting for TXS callback.
> */
> idr_remove(&wcid->pktid, cb->pktid);
> - __mt76_tx_status_skb_done(dev, skb, MT_TX_CB_TXS_FAILED |
> - MT_TX_CB_TXS_DONE, list);
I guess it is more readable as it was before.
Regards,
Lorenzo
> + __mt76_tx_status_skb_done(dev, skb2, MT_TX_CB_TXS_FAILED |
> + MT_TX_CB_TXS_DONE, list);
> }
> + wcid->last_idr_check_at = jiffies;
>
> out:
> if (idr_is_empty(&wcid->pktid))
> --
> 2.20.1
>
Attachment:
signature.asc
Description: PGP signature
