| Message ID | 1348772354-15936-5-git-send-email-bzhao@marvell.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote: > + if (associated && !tx_empty) { > + if (unlikely(local->scan_req->flags & > + CFG80211_SCAN_FLAG_LOW_PRIORITY)) I don't really see value in the "unlikely()" here, that just clutters the code and probably has very little effect on the runtime behaviour, this is very infrequently executed. > + case SCAN_ABORT: > + aborted = true; > + goto out_complete; Maybe we should have different flags though ... I mean, this is the first implementation that I hear of that interprets a background scan as "ok to abort at any time"? It seems very unlikely that other implementations would do that. I *think* (like I said before, I don't really know) that ours (Intel's) will just shorten the dwell time, or similar instead. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 28, 2012 at 4:06 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote: > >> + if (associated && !tx_empty) { >> + if (unlikely(local->scan_req->flags & >> + CFG80211_SCAN_FLAG_LOW_PRIORITY)) > > I don't really see value in the "unlikely()" here, that just clutters > the code and probably has very little effect on the runtime behaviour, > this is very infrequently executed. > > >> + case SCAN_ABORT: >> + aborted = true; >> + goto out_complete; > > Maybe we should have different flags though ... I mean, this is the > first implementation that I hear of that interprets a background scan as > "ok to abort at any time"? It seems very unlikely that other > implementations would do that. I *think* (like I said before, I don't > really know) that ours (Intel's) will just shorten the dwell time, or > similar instead. Well previous implementations I've done have used this technique for many years and you can find them in various products (assuming they haven't been totally rewritten) :-) -Sam -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2012-09-28 at 13:38 -0700, Sam Leffler wrote: > > Maybe we should have different flags though ... I mean, this is the > > first implementation that I hear of that interprets a background scan as > > "ok to abort at any time"? It seems very unlikely that other > > implementations would do that. I *think* (like I said before, I don't > > really know) that ours (Intel's) will just shorten the dwell time, or > > similar instead. > > Well previous implementations I've done have used this technique for > many years and you can find them in various products (assuming they > haven't been totally rewritten) :-) :-) You convinced me before though with the abort/not-abort notification being sufficient, so I think we can do with the low-priority flag and any kind of implementation. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 8c80455..5113a4c 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -821,6 +821,7 @@ enum { * @SCAN_SUSPEND: Suspend the scan and go back to operating channel to * send out data * @SCAN_RESUME: Resume the scan and scan the next channel + * @SCAN_ABORT: Abort the scan and go back to operating channel */ enum mac80211_scan_state { SCAN_DECISION, @@ -828,6 +829,7 @@ enum mac80211_scan_state { SCAN_SEND_PROBE, SCAN_SUSPEND, SCAN_RESUME, + SCAN_ABORT, }; struct ieee80211_local { diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c index c4cdbde..f95bc52 100644 --- a/net/mac80211/scan.c +++ b/net/mac80211/scan.c @@ -462,6 +462,7 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata, sizeof(*local->hw_scan_req) + req->n_channels * sizeof(req->channels[0]); local->hw_scan_req->ie = ies; + local->hw_scan_req->flags = req->flags; local->hw_scan_band = 0; @@ -562,6 +563,7 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local, unsigned long min_beacon_int = 0; struct ieee80211_sub_if_data *sdata; struct ieee80211_channel *next_chan; + enum mac80211_scan_state next_scan_state; /* * check if at least one STA interface is associated, @@ -620,10 +622,19 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local, usecs_to_jiffies(min_beacon_int * 1024) * local->hw.conf.listen_interval); - if (associated && (!tx_empty || bad_latency || listen_int_exceeded)) - local->next_scan_state = SCAN_SUSPEND; - else - local->next_scan_state = SCAN_SET_CHANNEL; + if (associated && !tx_empty) { + if (unlikely(local->scan_req->flags & + CFG80211_SCAN_FLAG_LOW_PRIORITY)) + next_scan_state = SCAN_ABORT; + else + next_scan_state = SCAN_SUSPEND; + } else if (associated && (bad_latency || listen_int_exceeded)) { + next_scan_state = SCAN_SUSPEND; + } else { + next_scan_state = SCAN_SET_CHANNEL; + } + + local->next_scan_state = next_scan_state; *next_delay = 0; } @@ -794,6 +805,9 @@ void ieee80211_scan_work(struct work_struct *work) case SCAN_RESUME: ieee80211_scan_state_resume(local, &next_delay); break; + case SCAN_ABORT: + aborted = true; + goto out_complete; } } while (next_delay == 0);