On 2011-03-10 10:55 PM, Mark Mentovai wrote:
> Felix Fietkau wrote:
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> [...]
>> static void ath9k_flush(struct ieee80211_hw *hw, bool drop)
>> {
>> struct ath_softc *sc = hw->priv;
>> + int timeout = 200; /* ms */
>> + int i, j;
>>
>> + ath9k_ps_wakeup(sc);
>> mutex_lock(&sc->mutex);
>>
>> cancel_delayed_work_sync(&sc->tx_complete_work);
>>
>> + if (drop)
>> + timeout = 1;
>>
>> + for (j = 0; j < timeout; j++) {
>> + int npend = 0;
>> + for (i = 0; i < ATH9K_NUM_TX_QUEUES; i++) {
>> + if (!ATH_TXQ_SETUP(sc, i))
>> + continue;
>>
>> + npend += ath9k_has_pending_frames(sc, &sc->tx.txq[i]);
>> }
>> +
>> + if (!npend)
>> + goto out;
>> +
>> + usleep_range(1000, 2000);
>> }
>>
>> + if (!ath_drain_all_txq(sc, false))
>> ath_reset(sc, false);
>>
>> +out:
>> ieee80211_queue_delayed_work(hw, &sc->tx_complete_work, 0);
>> mutex_unlock(&sc->mutex);
>> + ath9k_ps_restore(sc);
>> }
>
> My only comment here is that you still hit the sleep even on the last
> pass through the loop when it isn’t strictly necessary. Practically,
> the only place this would be realized is the case where |drop| is set.
> In this scenario, the code formerly didn’t sleep at all, but now can
> sleep for 1-2ms. I couldn’t find any calls to drv_flush with |drop|
> set, though, so it might just be an academic concern.
>
> The delay loops in v2 patches 2/4 and 4/4 also share this
> characteristic (although with shorter timeouts, and delays instead of
> sleeps). Have you considered restructuring these loops to avoid the
> final waits?
>
> This is admittedly a pretty minor thing.
Makes sense, I'll send a v3.
- Felix
--
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
