All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kan Yan <kyan@google.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: "Toke Høiland-Jørgensen" <toke@toke.dk>,
	linux-wireless@vger.kernel.org,
	make-wifi-fast@lists.bufferbloat.net,
	"Rajkumar Manoharan" <rmanohar@codeaurora.org>,
	"Felix Fietkau" <nbd@nbd.name>
Subject: Re: [PATCH RFC v3 0/4] Move TXQ scheduling into mac80211
Date: Wed, 12 Sep 2018 21:10:30 -0700	[thread overview]
Message-ID: <CA+iem5uyGOP1R-U_VxOjxUvy5CJV9HvxgrjciGG43Oi=SB-Hig@mail.gmail.com> (raw)
In-Reply-To: <1536578789.3224.69.camel@sipsolutions.net>

> I think this is a great approach, that we should definitely adopt in
> mac80211. However, I'm not sure the outstanding airtime limiting should
> be integrated into the fairness scheduling, since there are drivers such
> as ath9k that don't need it.
>
> Rather, I'd propose that we figure out the API for fairness scheduling
> first, and add the BQL-like limiter as a separate layer. They can be
> integrated in the sense that the limiter's estimate of airtime can be
> used for fairness scheduling as well in the case where the
> driver/firmware doesn't have a better source of airtime usage.
>
> Does that make sense?
Sure that make sense. Yes, the airtime based BQL like queue limiter is
not needed for drivers such as ath9k. We could leave the outstanding
airtime accounting and queue depth limit to another subject and get
airtime fairness scheduling API done first.


On Mon, Sep 10, 2018 at 4:26 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2018-09-10 at 13:17 +0200, Toke H=C3=B8iland-J=C3=B8rgensen wrote=
:
>
>> > > - I did not add any locking around next_txq(); the driver is still s=
upposed
>> > >   to maintain a lock that prevents two threads from trying to schedu=
le the
>> > >   same AC at the same time. This is what drivers already do, so I fi=
gured it
>> > >   was easier to just keep it that way rather than do it in mac80211.
>> >
>> > I'll look at this in the code, but from a maintainer perspective I'm
>> > somewhat worried that this will lead to issues that are really the
>> > driver's fault, but surface in mac80211. I don't know how easy it
>> > would be to catch that.
>>
>> Yeah, I get what you mean. The alternative would be to have a
>> ieee80211_start_schedule(ac) and ieee80211_end_schedule(ac), which
>> basically just takes a lock.
>
> And I guess start would increment the schedule number, which is now
> dependent on first
>
>> Would mean we could get rid of the 'first'
>> parameter for next_txq(), so might not be such a bad idea;
>
> Right, that's what I meant.
>
>> and if the
>> driver has its own locking the extra locking in mac80211 would just be
>> an always-uncontested spinlock, which shouldn't be much overhead, right?
>
> It may still bounce around CPUs if you call this from other places, but
> I suspect that wouldn't be the biggest issue. There are a lot of
> calculations going on too...
>
> johannes

  reply	other threads:[~2018-09-13  9:18 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-07 22:22 [PATCH RFC v3 0/4] Move TXQ scheduling into mac80211 Toke Høiland-Jørgensen
2018-09-07 22:22 ` [PATCH RFC v3 2/4] mac80211: Add airtime accounting and scheduling to TXQs Toke Høiland-Jørgensen
2018-09-10  8:18   ` Johannes Berg
2018-09-10 11:13     ` Toke Høiland-Jørgensen
2018-09-10 11:22       ` Johannes Berg
2018-09-12  0:07       ` Rajkumar Manoharan
2018-09-12 11:10         ` Toke Høiland-Jørgensen
2018-09-12 16:23           ` Rajkumar Manoharan
2018-09-07 22:22 ` [PATCH RFC v3 1/4] mac80211: Add TXQ scheduling API Toke Høiland-Jørgensen
2018-09-10  7:48   ` Johannes Berg
2018-09-10 10:57     ` Toke Høiland-Jørgensen
2018-09-10 11:03       ` Johannes Berg
2018-09-10 12:39         ` Toke Høiland-Jørgensen
2018-09-10 12:46           ` Johannes Berg
2018-09-10 13:08             ` Toke Høiland-Jørgensen
2018-09-10 13:10               ` Johannes Berg
2018-09-10 13:18                 ` Toke Høiland-Jørgensen
2018-09-10 14:51                   ` Johannes Berg
2018-09-10 15:00                     ` Toke Høiland-Jørgensen
2018-09-11  9:20                       ` Johannes Berg
2018-09-11  9:48                         ` Toke Høiland-Jørgensen
2018-09-10  8:04   ` Johannes Berg
2018-09-10 11:02     ` Toke Høiland-Jørgensen
2018-09-10 11:12       ` Johannes Berg
2018-09-07 22:22 ` [PATCH RFC v3 4/4] ath9k: Switch to mac80211 TXQ scheduling and airtime APIs Toke Høiland-Jørgensen
2018-09-07 22:22 ` [PATCH RFC v3 3/4] cfg80211: Add airtime statistics and settings Toke Høiland-Jørgensen
2018-09-10  8:23   ` Johannes Berg
2018-09-10 11:15     ` Toke Høiland-Jørgensen
2018-09-09 22:08 ` [PATCH RFC v3 0/4] Move TXQ scheduling into mac80211 Kan Yan
2018-09-10 10:52   ` Toke Høiland-Jørgensen
2018-09-10  7:46 ` Johannes Berg
2018-09-10 11:16   ` Toke Høiland-Jørgensen
2018-09-10 11:24     ` Johannes Berg
2018-09-10  7:52 ` Johannes Berg
2018-09-10 11:17   ` Toke Høiland-Jørgensen
2018-09-10 11:26     ` Johannes Berg
2018-09-13  4:10       ` Kan Yan [this message]
2018-09-13  9:25         ` Toke Høiland-Jørgensen

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='CA+iem5uyGOP1R-U_VxOjxUvy5CJV9HvxgrjciGG43Oi=SB-Hig@mail.gmail.com' \
    --to=kyan@google.com \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=make-wifi-fast@lists.bufferbloat.net \
    --cc=nbd@nbd.name \
    --cc=rmanohar@codeaurora.org \
    --cc=toke@toke.dk \
    /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.