linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Buesch <mb@bu3sch.de>
To: Michael Wu <flamingice@sourmilk.net>
Cc: Jiri Benc <jbenc@suse.cz>, John Linville <linville@tuxdriver.com>,
	linux-wireless@vger.kernel.org
Subject: Re: [PATCH RFC] mac80211: Make stop_queues() usable
Date: Tue, 3 Jul 2007 19:36:23 +0200	[thread overview]
Message-ID: <200707031936.24084.mb@bu3sch.de> (raw)
In-Reply-To: <200707031015.23890.flamingice@sourmilk.net>

On Tuesday 03 July 2007 19:15:19 Michael Wu wrote:
> On Tuesday 03 July 2007 01:39, Michael Buesch wrote:
> > On Tuesday 03 July 2007 06:09:19 Michael Wu wrote:
> > > On Monday 02 July 2007 13:35, Michael Buesch wrote:
> > > > +	netif_tx_lock_bh(mdev);
> > > >  	for (i = 0; i < hw->queues; i++)
> > > >  		ieee80211_stop_queue(hw, i);
> > > > +	netif_tx_unlock_bh(mdev);
> > >
> > > Well, looks like this will break stopping all tx queues from the tx
> > > handler by deadlocking. It may be useless for bcm43xx to call
> > > ieee80211_stop_queue, but there are other drivers which rely on it.
> >
> > Nobody said that. Of course bcm43xx needs stop_queue, too.
> >
> I mean stop_queues, in the tx path, which this patch would break.

It's useless to stop all queues from the TX handler.
If a queue overflows we should just stop _that_ one.


> > No, that is not enough. For example for switching bands we need to
> > reinitialize the board completely. Before I take the board down I must
> > ensure that no traffic is going on any longer.
> >
> Ok sure.
> 
> However, ensuring that there is no attempt to TX while the channel is being 
> changed is generally good so stopping the queues before every channel change 
> in mac80211 would be preferred.

Yes. That may also be a mac80211 issue.
The driver should really be able to completely stop TX, too.
Otherwise it is forced to implement ugly locks and workarounds
to prevent concurrency issues. I'm not sure if it's easily possible
to workaround without races.

> > > Of course, the ieee80211_stop_queue deadlock should still get fixed
> > > eventually..
> >
> > That's not needed, as it's only sane to call in the TX handler, where
> > it can't deadlock.
> By stop_queue, I mean stop_queues, which you do seem to want to call outside 
> of the tx_handler.

the _only_ place where it's sane to call stop_queues (to stop all queues)
is outside of the TX handler. Why would you stop all queues inside of
the TX path?

-- 
Greetings Michael.

  reply	other threads:[~2007-07-03 17:37 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-02 20:35 [PATCH RFC] mac80211: Make stop_queues() usable Michael Buesch
2007-07-03  4:09 ` Michael Wu
2007-07-03  8:39   ` Michael Buesch
2007-07-03 12:31     ` Patrick McHardy
2007-07-03 12:39       ` Michael Buesch
2007-07-03 12:56         ` Patrick McHardy
2007-07-03 17:15     ` Michael Wu
2007-07-03 17:36       ` Michael Buesch [this message]
2007-07-03 17:41         ` Michael Wu
2007-07-03 17:47           ` Michael Buesch

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=200707031936.24084.mb@bu3sch.de \
    --to=mb@bu3sch.de \
    --cc=flamingice@sourmilk.net \
    --cc=jbenc@suse.cz \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).