All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@toke.dk>
To: Johannes Berg <johannes@sipsolutions.net>,
	make-wifi-fast@lists.bufferbloat.net,
	linux-wireless@vger.kernel.org
Subject: Re: [RFC 1/3] mac80211: Add TXQ scheduling API
Date: Wed, 11 Oct 2017 16:06:41 +0200	[thread overview]
Message-ID: <87tvz5pymm.fsf@toke.dk> (raw)
In-Reply-To: <1507711498.1998.18.camel@sipsolutions.net>

Johannes Berg <johannes@sipsolutions.net> writes:

>> Well I was trying to do The Right Thing(tm), obviously ;)
>
> :-)
>
>> The driver_buffered field comes from the previous behaviour of ath9k:
>> It would basically set the station_buffered flag if there was
>> something in the retry queue at the time it goes to sleep. And on
>> wakeup, it will reschedule the txq if it has anything in the retry
>> queue. And, well, it just seemed odd that there was this duplicated
>> logic in the driver and mac80211, if the driver could just signal to
>> mac80211 to reschedule for it...
>
> Ok. I'm not sure what ath9k does, but mac80211 makes a distinction
> between "do I have something buffered" and "does the driver have
> something buffered".
>
> When the station wakes up it's pretty easy, we tell the driver and it
> just has to send its own frames first.
>
> But when we get a PS-Poll/U-APSD it's more complicated, and we have to
> tell the driver "please release N frames you have buffered" or we may
> have to tell it "please allow me to send N frames that I have buffered"
> which is why we need the distinction of whether or not the driver has
> something buffered.
>
> Ultimately, with TXQs, I hope this distinction can go away because the
> driver wouldn't buffer all by itself.

Yes, that would be good. Okay, I'll keep PS buffering the way it works
now.

>> But I guess I was really just getting ahead of myself there; so the
>> right thing to do for now is to keep the old behaviour, and then fix
>> it properly afterwards?
>
> I guess that'd be easier.
>
>> Yes, this makes sense. Each sleep period is pretty short, right? 
>
> Not necessarily.
>
>> I.e., we don't need to deal with interactions between CoDel and the
>> queue being stopped for a long period of time?
>
> I don't know enough about the possible interactions to answer that.
> Sleep periods may be long, though typically if there's traffic for
> stations then they want to retrieve that and will wake up relatively
> soon, but "relatively soon" may still be on the order of hundreds of
> milliseconds (or even seconds) because sometimes clients will only
> listen to DTIM beacons (DTIM period * beacon interval), and e.g. with
> WNM sleep mode it becomes even longer.

Ah. Right. Well, guess we'll have to cross that bridge once we get
there. Not sure what the right thing to do from the point of view of an
AQM in this instance. But at least making sure it doesn't drop the
packet at the head of every queue at wakeup would probably be good.

>> I was envisioning that next_txq() could also make those kinds of
>> decisions (i.e., preempt the normal scheduling algorithm when a
>> "special" TXQ needs to be scheduled immediately). But not sure if
>> that is enough for this case?
>
> Hmm, not sure. We really want this to be scheduled pretty much
> immediately because the other side will be waiting for the frames, and
> if we don't get an answer out quickly it'll have to assume we're
> broken. I don't know what the limit here is for our firmware, but we
> should really get this out as soon as possible in this case.

OK. But presumably it can't preempt packets already pushed to the
hardware, right? So telling the driver to immediately schedule a packet,
and making sure that the txq it gets from next_txq() is the right one
should do the trick? But I guess it's a bit of a roundabout way, which
may not be worth it to avoid an extra callback...

-Toke

  reply	other threads:[~2017-10-11 14:06 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-10 14:02 [RFC 1/3] mac80211: Add TXQ scheduling API Toke Høiland-Jørgensen
2017-10-10 14:02 ` [RFC 2/3] ath9k: Move to mac80211 " Toke Høiland-Jørgensen
2017-10-10 15:54   ` Johannes Berg
2017-10-10 14:02 ` [RFC 3/3] ath10k: " Toke Høiland-Jørgensen
2017-10-10 15:53 ` [RFC 1/3] mac80211: Add " Johannes Berg
2017-10-10 17:51   ` Toke Høiland-Jørgensen
2017-10-11  8:46     ` Johannes Berg
2017-10-11 13:54       ` Toke Høiland-Jørgensen
2017-10-10 16:05 ` Johannes Berg
2017-10-10 18:04   ` Toke Høiland-Jørgensen
2017-10-11  8:44     ` Johannes Berg
2017-10-11 14:06       ` Toke Høiland-Jørgensen [this message]
2017-10-11 14:15         ` Johannes Berg
2017-10-11 14:29           ` Toke Høiland-Jørgensen
2017-10-11 14:34             ` Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87tvz5pymm.fsf@toke.dk \
    --to=toke@toke.dk \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=make-wifi-fast@lists.bufferbloat.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.