On Thu, Jan 09, 2014 at 04:26:39AM +0400, Gleb Smirnoff wrote: > Eric, > > On Tue, Jan 07, 2014 at 10:26:20PM +0000, Eric Davis wrote: > E> Author: edavis > E> Date: Tue Jan 7 22:26:20 2014 > E> New Revision: 260415 > E> URL: http://svnweb.freebsd.org/changeset/base/260415 > E> > E> Log: > E> defragment mbuf chains longer than hw segment limit before dropping > E> > E> Approved by: davidch > E> > E> Modified: > E> head/sys/dev/bxe/bxe.c > E> head/sys/dev/bxe/ecore_hsi.h > E> > E> Modified: head/sys/dev/bxe/bxe.c > E> > ============================================================================== > E> --- head/sys/dev/bxe/bxe.c Tue Jan 7 21:25:18 2014 (r260414) > E> +++ head/sys/dev/bxe/bxe.c Tue Jan 7 22:26:20 2014 (r260415) > E> @@ -34,7 +34,7 @@ > E> #include <sys/cdefs.h> > E> __FBSDID("$FreeBSD$"); > E> > E> -#define BXE_DRIVER_VERSION "1.78.76" > E> +#define BXE_DRIVER_VERSION "1.78.77" > E> > E> #include "bxe.h" > E> #include "ecore_sp.h" > E> @@ -5501,10 +5501,31 @@ bxe_tx_encap(struct bxe_fastpath *fp, st > E> fp->eth_q_stats.tx_window_violation_std++; > E> } > E> > E> - /* XXX I don't like this, change to double copy packet */ > E> + /* lets try to defragment this mbuf */ > E> + fp->eth_q_stats.mbuf_defrag_attempts++; > E> > E> - /* no sense trying to defrag again, just drop the frame */ > E> - rc = ENODEV; > E> + m0 = m_defrag(*m_head, M_DONTWAIT); > > It is already more than a decade as M_DONTWAIT shouldn't be used. Generic > malloc(9) flag of M_NOWAIT should be used. > > I wonder, why did you use it in new code? Is that just through habit, > or do we still got somewhere outdated piece of documentation?
I did not know that. The new bxe driver did leverage some code from the
older one and this block is definitely a copy-paste-and-tweak.
I'll be sure to update those defines and make sure only M_WAITOK and
M_NOWAIT is used by the driver.
FWIW, I see there are a couple other drivers under head/sys/dev that are
still using M_DONTWAIT as well.
- e
> E> + if (m0 == NULL) {
> E> + fp->eth_q_stats.mbuf_defrag_failures++;
> E> + /* Ugh, just drop the frame... :( */
> E> + rc = ENOBUFS;
> E> + } else {
> E> + /* defrag successful, try mapping again */
> E> + *m_head = m0;
> E> + error = bus_dmamap_load_mbuf_sg(fp->tx_mbuf_tag,
> E> + tx_buf->m_map, m0,
> E> + segs, &nsegs, BUS_DMA_NOWAIT);
> E> + if (error) {
> E> + fp->eth_q_stats.tx_dma_mapping_failure++;
> E> + /* No sense in trying to defrag/copy chain, drop it. :( */
> E> + rc = error;
> E> + }
> E> +
> E> + /* if the chain is still too long then drop it */
> E> + if (__predict_false(nsegs > 12)) {
> E> + rc = ENODEV;
> E> + }
> E> + }
> E> }
>
> --
> Totus tuus, Glebius.
pgpcCTjXaUXC4.pgp
Description: PGP signature
