On Tue, 2010-02-02 at 10:54 -0800, Reinette Chatre wrote: > mac80211 will defer the handling of scan requests if it is busy > with management work at the time. The scan requests are > deferred and run after the work has completed. When this occurs > there are currently two problems. > > * The scan request for hardware scan is not fully populated with > the band and channels to scan not initialized. > * When the scan is queued the state is not correctly updated to > reflect that a scan is in progress. The problem here is that when > the driver completes the scan and calls ieee80211_scan_completed() > a warning will be triggered since mac80211 was not aware that a scan > was in progress. Good analysis, thanks. > Both issues are fixed here by first ensuring that scan request is fully > initialized before it is queued and next by setting the scanning state > correctly before it is queued for execution. > * I would like to confirm one change I made in ieee80211_work_work. From > what I can tell software scanning does not expect the scanning state to > be set in ieee80211_scan_work, but hardware scanning does. Is this > correct? I think so, but I kinda think it makes that code rely too much on the internals. > * This is a proposed fix for > http://bugzilla.kernel.org/show_bug.cgi?id=15119, which is tracked as a > regression. The user is having some system problems and is not able to > test it. > > * This is an important fix to get in, otherwise wireless will become a top > performer on kerneloops.org. Well said ;) Thanks to your analysis of the problem, and the remain-on-channel work, I was able to reproduce the problem, and the following seems to fix it for me; what do you think? johannes --- wireless-testing.orig/net/mac80211/scan.c 2010-02-02 20:37:28.000000000 +0100 +++ wireless-testing/net/mac80211/scan.c 2010-02-02 20:39:35.000000000 +0100 @@ -345,6 +345,14 @@ static int __ieee80211_start_scan(struct if (local->scan_req) return -EBUSY; + local->scan_req = req; + local->scan_sdata = sdata; + + if (!list_empty(&local->work_list)) { + /* wait for the work to finish/time out */ + return 0; + } + if (local->ops->hw_scan) { u8 *ies; @@ -353,8 +361,11 @@ static int __ieee80211_start_scan(struct req->n_channels * sizeof(req->channels[0]) + 2 + IEEE80211_MAX_SSID_LEN + local->scan_ies_len + req->ie_len, GFP_KERNEL); - if (!local->hw_scan_req) + if (!local->hw_scan_req) { + local->scan_req = NULL; + local->scan_sdata = NULL; return -ENOMEM; + } local->hw_scan_req->ssids = req->ssids; local->hw_scan_req->n_ssids = req->n_ssids; @@ -366,14 +377,6 @@ static int __ieee80211_start_scan(struct local->hw_scan_band = 0; } - local->scan_req = req; - local->scan_sdata = sdata; - - if (!list_empty(&local->work_list)) { - /* wait for the work to finish/time out */ - return 0; - } - if (local->ops->hw_scan) __set_bit(SCAN_HW_SCANNING, &local->scanning); else -- 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
