On Thu, Oct 07, 2010 at 11:24:46AM +0200, Johannes Berg wrote:
> On Thu, 2010-10-07 at 11:18 +0200, Stanislaw Gruszka wrote:
>
> > > --- wireless-testing.orig/net/mac80211/scan.c 2010-10-07 10:27:37.000000000 +0200
> > > +++ wireless-testing/net/mac80211/scan.c 2010-10-07 10:31:19.000000000 +0200
> > > @@ -693,8 +693,6 @@ void ieee80211_scan_work(struct work_str
> > > goto out_complete;
> > > }
> > >
> > > - mutex_unlock(&local->mtx);
> > > -
> > > /*
> > > * as long as no delay is required advance immediately
> > > * without scheduling a new work
> > > @@ -725,6 +723,7 @@ void ieee80211_scan_work(struct work_str
> > > } while (next_delay == 0);
> > >
> > > ieee80211_queue_delayed_work(&local->hw, &local->scan_work, next_delay);
> > > + mutex_unlock(&local->mtx);
> > > return;
> > >
> > > out_complete:
> >
> > We never keep locking inside do { } while (next_delay == 0) loop,
> > perhaps below patch is something better:
>
> Yeah, we didn't have that before, but weren't you yourself arguing that
> dropping and re-acquiring could lead to issues? :)
Yep.
> FWIW, I've looked at the code and all we do is possibly acquire the
> iflist_mtx (which is fine under local->mtx) and do some driver calls,
> which should also be fine.
>
> So what do you think?
Let's fix with your patch. I'm going to test it (on hw scan :/ at least).
Stanislaw
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
