On Sat, Nov 03, 2012 at 06:56:43PM +0100, Arend van Spriel wrote: > On 10/26/2012 04:23 PM, Seth Forshee wrote: > > I've been looking into the issues with brcmsmac performance reported at > > [0] and [1]. I started out looking into the tx queueing based on the "No > > where to go" messages in the logs. This code has a number of > > shortcomings: > > > > - The amount of bufferring is excessive. The tx queue will buffer up to > > 228 packets, and each of the tx DMA rings will queue up to 256 more. > > > > - There's no flow control. If the queue fills up packets begin to get > > dropped, as evidenced by the "No where to go" messages. > > > > - Without flow control the tx queue probably helps avoid dropping > > packets for short bursts due to the sheer number of packets that will > > be buffered, but if flow control is added the only remaining benefit > > that I can see is that it accumulates packets for aggregation. The tx > > queue is far more complex than needed for supporting aggergation, > > however. > > > > As a result I worked up the following patches to add flow control remove > > the tx queue. > > > > These patches change the tx handler to directly hand off packets to the > > DMA code. The convoluted priority->precedence->fifo mapping is converted > > to a simple one-to-one mapping of the mac80211 queues to fifos. Non- > > aggregate frames are immediately inserted into the DMA ring. > > I mentioned earlier we were still looking at the first patch in this > series and do more testing on it. However, I wanted to provide feedback > on the other patches now. See below. > > > Thanks, > > Seth > > > > [0] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1046507 > > [1] http://www.spinics.net/lists/linux-wireless/msg96287.html > > > > > > Seth Forshee (18): > > brcmsmac: Rework tx code to avoid internal buffering of packets > > under test > > > brcmsmac: Use correct descriptor count when calculating next rx > > descriptor > > nice catch. acked. > > > brcmsmac: Reduce number of entries in tx DMA rings > > under test. > > > brcm80211: Allow trace support to be enabled separately from debug > > A bit confusing with BRCMDBG selecting BRCMS_TRACING. I did this so that the effect of selecting BRCMDBG was unchanged, since currently tracing is enabled by BRCMDBG. I'm happy to change it so that BRCMDBG does not select BRCMS_TRACING if you prefer. > > brcm80211: Add macro for checking if debug log levels are enabled > > During mainlining we got rid of such a macro. Strange to put something > similar back in there. I wasn't aware of that. I personally prefer the macro, but I can change it back to open coding the checks. > > brcm80211: Convert log message levels to debug levels > > acked. > > > brcmsmac: Add module parameter for setting the debug level > > I would prefer doing this through debugfs. Okay, I'll change that for v2. > > brcmsmac: Add support for writing debug messages to the trace buffer > > can you name the new files just debug.[ch] I'll change this for v2 as well. > > brcmsmac: Use debug macros for general error and debug statements > > acked. > > > brcmsmac: Add BRCMS_DBG_MAC80211 debug macro > > brcmsmac: Add RX and TX debug macros > > brcmsmac: Add INT debug macro > > brcmsmac: Add DMA debug macro > > brcmsmac: Add HT debug macro > > I would prefer the macros to be in lower case. Will do. > > brcmsmac: Improve tx trace and debug support > > acked. > > > brcmsmac: Add tracepoint for macintstatus > > useful one. acked. > > > brcmsmac: Add tracepoint for AMPDU session information > > acked. > > > brcmsmac: Remove some noisy but uninformative debug messages > > acked. I would say: noisy *and* uninformative but that is just semantics ;-) Yes, "and" would be better here. Thanks for the feedback! Seth -- 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
