Re: [PATCH] bcm43xx: further fix for periodic work errors

[Michael Buesch <mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>]

On Saturday 23 September 2006 21:06, Larry Finger wrote:
> Michael Buesch wrote:
> > On Saturday 23 September 2006 06:08, Larry Finger wrote:
> >> Recent changes in the setup for preemptible periodic work fixed most
> >> of the problems with NETDEV watchdog timeouts; however, some variants
> >> of the bcm43xx device still had the problem. These were fixed by setting
> >> the parameter MAXIMUM_BADNESS to 0. By doing so, all the functionality
> >> associated with calculating the 'badness' of the upcoming periodic work
> >> is no longer needed; therefore it is removed.
> > 
> > Uhm, no. Wait. _Why_ does the watchdog trigger.
> > All periodic work in the fastpath (which you remove with this patch)
> > is supposed to execute in a few microseconds.
> > I don't think we want to fix this my removing the fastpath and always
> > taking the _expensive_ slowpath periodic work.
> > 
> > So why does the watchdog trigger for the fast periodic work?
> > We need to find out.
> > Removing the fastpath is just bad for overall latency.
> > 
> > The two fastpath periodic works are 15 and 30, if executed
> > standalone. If the 15 and/or 30 is execiuted alongside with
> > a 60sec work, it's all slowpath, of course.
> 
> I was thinking that the 15 second periodic work called mac suspend, which is the most expensive part 
> of the slowpath, but I see that is an unlikely condition. I'm now testing to see if moving the 
> netif_tx_disable/netif_wake_queue pair into all paths fixes the errors. Those calls should be 
> relatively inexpensive.

Well, even _if_ mac_suspend takes a few milliseconds (which it
does not), it would not trigger the watchdog.
I measured the time it takes to execute the various works
and based the badness selection on the results.

If the 15 or 30 second work is really able to trigger a watchdog
timeout, it's a _bug_ that needs to be fixed and not to be
papered over.
It won't trigger the watchdog, because it is running too long
uninterruptible (it won't run 5sec...). If it triggers, it's
triggered by something else (like the synchronize_net thingie
in the past).

-- 
Greetings Michael.