On 01/28/2011 05:24 AM, Johannes Berg wrote:
On Thu, 2011-01-27 at 09:17 -0800, Ben Greear wrote:
+ if (local->scan_channel) {
+ chan = local->scan_channel;
+ channel_type = NL80211_CHAN_NO_HT;
+ } else if (local->tmp_channel) {
+ chan = scan_chan = local->tmp_channel;
+ channel_type = local->tmp_channel_type;
+ } else {
+ chan = local->oper_channel;
+ channel_type = local->_oper_channel_type;
+ }
Don't understand -- why not return true in the else branch?
Because the hardware might not actually be set to the oper_channel.
The idea is that you configure the mac80211 state as you want it, and then
use this method to figure out if you really need to make hardware
changes.
Oh. Wouldn't it make more sense to stick that into the _config()
function then and return something there? Hmm. I kinda start to
understand I guess.
The code is similar..seems like it could be put into a common helper
method, but I haven't thought of a clean way to do that yet.
+ if (chan != local->oper_channel ||
+ channel_type != local->_oper_channel_type)
+ return false;
+
+ /* Check current hardware-config against oper_channel. */
+ if ((local->oper_channel != local->hw.conf.channel) ||
+ (local->_oper_channel_type != local->hw.conf.channel_type))
+ return false;
That's confusing, and kinda racy IIRC?
This method should be locked such that the hardware conf
cannot be changed while it is being called. I can double
check that this is true.
Not all of this is always properly locked unfortunately. Not sure about
this case though.
On that note: It seems to me that __ieee80211_scan_completed_finish
must grab the mutex_lock(&local->mtx); early in this method, before
any calls to ieee80211_hw_config, for instance. I believe this would
be an issue in both my patch and the existing code. I'm adding code
to grab it early, but still release after recalc_idle() is called.
I'll test it with lockdep enabled to make sure it's at least mostly
sane.
Also, won't this do some weird things like not stop, but try to start
stations again?
I was thinking that should be harmless. As far as I can tell, current
code would never actually stop beaconing in this method but might try
to start it later, so it must not cause too much trouble.
Yeah, maybe you're right and it doesn't matter, but I think it'd be
nicer to always nest the calls. I see you've done that already.
Yeah, the v5 patch was better about this. The code is still complex,
but perhaps we'll think of something simpler down the road.
I'll post a v6 soon, with the earlier mutex grab I mention above
and the debug stuff removed.
Hopefully it's getting close!
Thanks,
Ben
--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
--
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
