All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Arend van Spriel <arend@broadcom.com>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"John W. Linville" <linville@tuxdriver.com>
Subject: Re: rework on .flush() callback
Date: Mon, 19 Nov 2012 15:52:02 +0100	[thread overview]
Message-ID: <1353336722.18299.19.camel@jlt4.sipsolutions.net> (raw)
In-Reply-To: <50A67308.8020709@broadcom.com>

On Fri, 2012-11-16 at 18:08 +0100, Arend van Spriel wrote:
> On 11/16/2012 02:34 PM, Johannes Berg wrote:
> >> The problem here is that in brcmsmac the flush does the
> >> >ieee80211_wake_queues() call, because that could also wakeup the netif
> >> >queues. So doing it in the driver seems a bad idea. Any suggestion on
> >> >how to solve this?
> > Yeah so .. I actually thought about this at some point, it's tricky. For
> > the global queues we check what reasons we had for stopping them, but we
> > don't do that for the netif queues. Maybe we should? I also think flush
> > should at least have the option to be per queue, so that we could do
> > something per sdata in mac80211 if the driver uses different HW queues
> > for different interfaces.
> 
> To me so from driver perspective, it was not clear what 
> ieee80211_stop/wake_queues was operating on. Somehow the knowledge that 
> the netif queues are already stopped upon calling 
> ieee80211_stop_queues() should be retained.

Yes. From the driver perspective, those calls operate on the HW queues
(which you can modify now, using the IEEE80211_HW_QUEUE_CONTROL flag and
associated APIs). But we don't retain the netif queue state separately,
which I agree is a bug.

> Regarding the flush do you mean flush per queue or flush per vif? Per 
> vif could have its perks, I guess.

Not sure. With the IEEE80211_HW_QUEUE_CONTROL per queue might make
sense, and would actually make more sense than per vif in most cases.
However, it'd have to be a bitmap so the flush can happen in parallel.

> > However, I'm not sure I'll have time to work on corner cases with
> > software scanning since we don't use that. I might work on the flush
> > thing though, that could be interesting.
> 
> ok. The drv_flush() call is done in several places so not only scanning. 
> All with the drop flag set to false. Is that just to be prepared or do 
> you foresee an actual use-case?

I think at some point I thought we would use it, but if somebody is
going to change the flush callback (e.g. by passing a hw queue bitmap) I
would suggest to remove the drop flag.

johannes


      reply	other threads:[~2012-11-19 14:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-15 12:47 rework on .flush() callback Arend van Spriel
2012-11-16 13:34 ` Johannes Berg
2012-11-16 17:08   ` Arend van Spriel
2012-11-19 14:52     ` Johannes Berg [this message]

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=1353336722.18299.19.camel@jlt4.sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=arend@broadcom.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    /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.