On Thu, Jun 14, 2012 at 3:09 PM, Vitaly Wool <vitalywool@xxxxxxxxx> wrote: >>> I have some doubts about the following patch: "mac80211: quiesce vif >>> before suspending". In fact, I think it introduces a hard-to-track >>> bug: when a driver signals disconnection from the AP and the device >>> goes to sleep shortly thereafter, beacon_connection_loss_work will get >>> cancelled and the actual disconnection will never happen. Shouldn't we >>> fix that or is there something I'm missing? >>> >> >> i think that flush_workqueue(local->workqueue) should handle the >> normal disconnect-before-suspend case. >> however, there's probably still some race condition when the work is >> scheduled between the flush_workqueue() and the drv_suspend() call. >> i'm not sure what is the right solution regarding it. maybe add >> another flush_workqueue() after drv_suspend? > > Maybe, but aren't we increasing the probability of hitting the 3s > suspend timeout and thus crashing the system here? > flush_workqueue() will do it anyway (if the work was queued before), so i don't think it's much different. >> additionally, i don't think that the patch itself is the issue here, >> as without it beacon_connection_loss_work might run after mac80211 was >> already suspended, which might result in unexpected outcome as well... > > Well, the patch is the issue because previously the driver could just > stall the work and now it has no control over it. > > I think there are two opportunities to look into: > > 1. Freeze workqueues on suspend without flushing hmm... this might be a bit unexpected - userspace might want to use wowlan only while connected. so postponing the disconnection after the resume (while the ap is already gone) might be wrong. > 2. Reject suspend if an important work is pending (and yes, connection > loss work *is* important!) > i think this solution is easier and more correct. Eliad. -- 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
