linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Helmut Schaa <helmut.schaa@googlemail.com>
To: Zhu Yi <yi.zhu@intel.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	Jouni Malinen <j@w1.fi>
Subject: Re: ipw2100: race between isr_indicate_associated and rx path
Date: Tue, 3 Mar 2009 12:33:23 +0100	[thread overview]
Message-ID: <200903031233.23702.helmut.schaa@gmail.com> (raw)
In-Reply-To: <1235696139.6354.584.camel@debian>

Am Freitag, 27. Februar 2009 schrieb Zhu Yi:
> On Wed, 2009-02-25 at 20:39 +0800, Helmut Schaa wrote:
> > Argh! Just found out why dev_activate is called such late after
> > netif_carrier_on:
> > 
> > ipw2100 calls netif_carrier_on followed by netif_wake_queue when the
> > driver
> > moves from associating to associated state. netif_carrier_on will then
> > call linkwatch_fire_event. However the carrier_on event is not treated
> > as
> > urgent and as such the event is delayed (and thus dev_activate too).
> > 
> > An event is considered urgent if the netdev is running, has a carrier
> > _and_ any of the TX qdiscs changed. Since ipw2100 first calls
> > carrier_on,
> > the last condition is not met and thus the event is not considered
> > urgent and gets delayed.
> 
> Calling netif_wake_queue() before netif_carrier_on() is not correct in
> semantics. Even it works, it looks like a workaround hack. I still think
> making a sync version of netif_carrier_on is the way to go.

Hmm, we could use register_netdevice_notifier to get notified when the
carrier event is sent out and thus the tx queues are available.

The flow in ipw2100 would then be:

1) isr_indicate_associated:
- move to ASSOCIATING state
- start buffering incoming frames

2) ipw2100_ex_event_work:
- call carrier_on and wake_queues

3) netdevice_notification:
- move to ASSOCIATED state and replay buffered frames

I hope I'll find some time to try that soon.

Helmut

      reply	other threads:[~2009-03-03 11:33 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200901211734.48625.helmut.schaa@gmail.com>
     [not found] ` <200901271521.24395.helmut.schaa@gmail.com>
     [not found]   ` <200902051511.31268.helmut.schaa@gmail.com>
     [not found]     ` <200902231138.58067.helmut.schaa@gmail.com>
2009-02-24  3:34       ` [Ipw2100-devel] ipw2100: race between isr_indicate_associated and rx path Zhu Yi
2009-02-24 12:15         ` Helmut Schaa
2009-02-25  1:18           ` Zhu Yi
2009-02-25 12:39             ` Helmut Schaa
2009-02-27  0:55               ` Zhu Yi
2009-03-03 11:33                 ` Helmut Schaa [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=200903031233.23702.helmut.schaa@gmail.com \
    --to=helmut.schaa@googlemail.com \
    --cc=j@w1.fi \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=yi.zhu@intel.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).