Re: [PATCH] mac80211: fix some unforgotten items on suspend

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1322 bytes --]

On Fri, 2009-05-15 at 10:22 +0200, Johannes Berg wrote:

> > +++ b/net/mac80211/pm.c
> > @@ -18,6 +18,10 @@ int __ieee80211_suspend(struct ieee80211_hw *hw)
> >  
> >  	flush_workqueue(local->hw.workqueue);
> >  
> > +	/* Don't stuff our workqueue during suspend->resume cycle */
> > +	del_timer_sync(&local->dynamic_ps_timer);
> > +	cancel_work_sync(&local->dynamic_ps_enable_work);
> > +
> 
> ack, but you forgot to add a comment why dynamic_ps_disable_work doesn't
> need to be handled (actually it does need to handled with the current
> code but shouldn't be necessary... up for a challenge? -- why? and how
> to fix that better than handling it here?)
> 
> If you ever suspend in AP mode there's also an issue with each station's
> cleanup timer, and for mesh each station's plink timer.

Hmm, actually, this isn't safe. We might still be receiving frames at
this point, which would start the timer again.

> I don't think that helps, like I said the timers are still set up, you
> need to actually queue the work now. However that kinda sucks for small
> suspend times. Not really sure what to do.

Instead of doing the setup_timer dance you were doing, we should simply
do add_timer /iff/ del_timer_sync() returned 1, because that means we
actually cancelled the timer.

johannes


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]