On Wed, Feb 03, 2016 at 04:41:34PM +1300, Richard Procter wrote: > > On Tue, 2 Feb 2016, Stefan Sperling wrote: > > > On Sat, Jan 30, 2016 at 10:49:38PM +1300, Richard Procter wrote: > > > - ring->queued--; > > > + atomic_dec_int(&ring->queued); > > > > > - ring->queued += ntxds; > > > + atomic_add_int(&ring->queued, ntxds); > > > > I don't think these make a difference in the current way of things. > > Wireless drivers run interrupts under the kernel big lock, interrupts > > aren't preemptible, and AFAIK (most?) 32bit integer operations are atomic. > [...] > > Hmm. Taking a closer look, if_start() is already called under splnet. > > So adding splnet to rt2860_tx() shouldn't make a difference. > > You're right, the atomic is unnecessary. I'd assumed rt2860_tx() was > running under splsoftnet. I could have sworn I'd seen errors without the > atomic but that now tests fine, too. (I still see errors without the > ring->cur fix.) > > > This also means the card cannot interrupt in the way your comment > > describes, i.e. the problem you're "fixing" here cannot exist... ? > > Also right. > > This simplifies things --- see below for the patch minus the above. > Without it my card under stress sees 1 oerror per ~217 packets; it's > now sent 5E6 without seeing any. > > cheers, > Richard.
I think these changes are good. I'm testing them with 2 different rt2860 style ral(4) cards. I don't have the nice model you've got, though :-( Mine are 2GHz only which makes it a bit difficult to push packets out fast. There's a lot of noise from other networks. One more thing I'd like to ask: Could you transform the following paragraph you wrote into a comment somewhere in the code? > The new code limits the dmamap to cover at most 8 tx descriptors (= 15 > fragments): now, if an mbuf fits a dma-map it will fit any ring with at least > 8 free descriptors. So we need only check for 8 free descriptors and are > longer obliged to count fragments and jump hoops. The cost is unused tx > ring descriptors, at most 7 of 63, when a one-fragment mbuf occupies one > descriptor in the last block of 8.
