On Thu, May 14, 2009 at 5:32 PM, Luis R. Rodriguez <lrodriguez@xxxxxxxxxxx> wrote: > On Mon, May 11, 2009 at 12:09:22PM -0700, Luis Rodriguez wrote: >> On Mon, May 11, 2009 at 11:53:30AM -0700, Johannes Berg wrote: >> > On Mon, 2009-05-11 at 11:34 -0700, Luis R. Rodriguez wrote: >> > >> > > > Also, __ieee80211_queues_stopped_by_reason is misnamed since it checks >> > > > only a single queue. I also think that we can do this much better by >> > > > keeping track of the suspend state in a new variable rather than looking >> > > > at all the queues; even just checking queue 0 would be sufficient, but I >> > > > think a new variable is warranted. >> > > >> > > A global variable for going to suspend? Hm, do we have a system wide >> > > thing for this instead? >> > >> > No, and even then we couldn't use it I think since you can have >> > per-device suspend. I was just thinking of a local->variable. >> >> OK thanks will add this. > > So I've added the WARN_ON() here as we agreed to investigate on irc. Turns out > we hit it several times as expected. We hit it while going to suspend > once, and then on resume 3 times. The trace doesn't help much except for acknowledging the issue I was pointing out. The workqueue is run because we flush it, twice, during suspend. The idle recalc is run because we call it within ieee80211_sta_work(). If we want we can move the check for local->suspended to ieee80211_sta_work() but then we'd have to set local->suspended early on during suspend before the first flush and that doesn't seem right. So I think the patch as I have it is fine unless I'm missing something. Luis -- 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
