On 2016-06-17 15:43, Toke Høiland-Jørgensen wrote:
> Felix Fietkau <nbd@xxxxxxxx> writes:
>
>> On 2016-06-17 11:09, Toke Høiland-Jørgensen wrote:
>>> This patch leaves the code for ath9k's internal per-node per-tid
>>> queues in place and just modifies the driver to also pull from
>>> the new mac80211 intermediate software queues, and implements
>>> the .wake_tx_queue method, which will cause mac80211 to deliver
>>> packets to be sent via the new intermediate queue.
>>>
>>> Signed-off-by: Tim Shepard <shep@xxxxxxxxxxxx>
>>>
>>> Reworked to not require the global variable renaming in ath9k.
>>>
>>> Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxx>
>>> ---
>>> drivers/net/wireless/ath/ath9k/ath9k.h | 16 +++-
>>> drivers/net/wireless/ath/ath9k/debug_sta.c | 7 +-
>>> drivers/net/wireless/ath/ath9k/init.c | 1 +
>>> drivers/net/wireless/ath/ath9k/main.c | 1 +
>>> drivers/net/wireless/ath/ath9k/xmit.c | 119 +++++++++++++++++++++++++----
>>> 5 files changed, 125 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>>> index 93b3793..caeae10 100644
>>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>>> @@ -145,8 +145,6 @@ int ath_descdma_setup(struct ath_softc *sc, struct ath_descdma *dd,
>>> #define BAW_WITHIN(_start, _bawsz, _seqno) \
>>> ((((_seqno) - (_start)) & 4095) < (_bawsz))
>>>
>>> -#define ATH_AN_2_TID(_an, _tidno) (&(_an)->tid[(_tidno)])
>>> -
>>> #define IS_HT_RATE(rate) (rate & 0x80)
>>> #define IS_CCK_RATE(rate) ((rate >= 0x18) && (rate <= 0x1e))
>>> #define IS_OFDM_RATE(rate) ((rate >= 0x8) && (rate <= 0xf))
>>> @@ -232,8 +230,10 @@ struct ath_buf {
>>>
>>> struct ath_atx_tid {
>>> struct list_head list;
>>> + struct sk_buff_head i_q;
>> Do we really need a third queue here? Instead of adding yet another
>> layer of queueing here, I think we should even get rid of buf_q.
>
> This is definitely something that needs to be improved. One other
> sticking point related to this: in the current version of this patch
> ath_tid_has_buffered() gains a side effect of pulling from the mac80211
> txq, which is obviously not so nice.
>
> The obvious way to get rid of this is to export a txq_has_buffered()
> function at the mac80211 layer. But avoiding that may be possible; the
> sticking point is what to do with the code paths that do not dequeue
> packets, but check ath_tid_has_buffered() to decide whether to schedule
> the queue and/or to tell ieee80211_sta_set_buffered() about it (these
> are for instance ath_tx_aggr_sleep/wakeup(). Can those just be removed
> (i.e. don't call into ieee80211, and always schedule the txq on wakeup?
> I'm not familiar enough with the intermediate queues to make that
> call...
For tx scheduling, we can use swq_nonempty and deal with false positives.
For power save we should only use ieee80211_sta_set_buffered if the
driver itself has buffered some frames. Indication of packets in the
mac80211 intermediate queue is already taken care of inside mac80211.
- 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
