Subject: Re: [PATCH] softmac: remove netif_tx_disable when scanning

From: Michael Buesch <[email-masked]>
Date: Sun, 26 Nov 2006 14:06:57 +0100

On Sunday 26 November 2006 14:05, Larry Finger wrote:
> Michael Buesch wrote:
> > On Sunday 26 November 2006 11:03, Johannes Berg wrote:
> >> On Sat, 2006-11-25 at 23:05 -0500, Daniel Drake wrote:
> >>
> >>> Surely disabling the queue actually makes sense, in order to avoid 
> >>> frames designated for the "current session" being transmitted on 
> >>> different channels during scanning?
> >> The problem is that queue disabling isn't refcounted so that a scan that
> >> collides with bcm43xx having disabled the queue for calibration might
> >> re-enable the queue while bcm43xx is still calibrating.
> >>
> >> Clearly, this doesn't fully fix the problem because softmac will try to
> >> transmit frames during the calibration. Hence, a proper fix would be to
> >> not remove the calls to netif_tx_disable but make them go through
> >> softmac (ieee80211_tx_disable) to make sure that softmac doesn't try to
> >> scan while the queues are disabled, which would fix the aforementioned
> >> problem of softmac enabling the queue while the driver needs it disabled
> >> for free.
> > 
> > Yeah, but I don't think it's worth fixing. ;)
> > Simply removing this tx_disable in softmac fixes a _big_ bug
> > and that must be applied. The bug of being able to scan while
> > calibrating is very minor and not worth to fix. Fixing this
> > will likely introduce other bugs. It's simply not worth it
> > anymore.
> 
> I actually found one of these while testing WX locking as suggested by Dan Williams. His method has 
> two scripts running at the same time with one of them doing an 'iwlist ethX scan' and the other 
> executing an 'iwconfig ethX' as fast as possible. Running this test for 36 hours failed to indicate 
> any locking errors, but running it without the patch under discussion led to a number of the 
> following errors:
> 
> kernel: bcm43xx: ASSERTION FAILED (!ring->suspended) at: 
> drivers/net/wireless/bcm43xx/bcm43xx_dma.c:71:request_slot()
> 
> This error _NEVER_ happened when softmac was not allowed to disable TX.

Yeah, that's the error I expected.
Nice that you proved my point for this being an important patch. ;)

-- 
Greetings Michael.