svn commit: r344817 - in head/sys: dev/e1000 net

[Eric Joyner]

I'm cc'ing Jake so that he can provide a response.

- Eric

On Tue, Mar 5, 2019 at 1:12 PM Rodney W. Grimes <freebsd at gndrsh.dnsmgr.net>
wrote:

> > This represents a misunderstanding of how defines are used. This left
> > the option open to the user to enable the use of larger than page size
> > buffers as it does enable better performance. Over the course of a
> > long uptime memory can get too fragmented. However, this left it open
> > to the end consumer.
> >
> > I'd like to see this reverted with perhaps a better name for the
> > define and the addition of an explanatory comment.
> > Thanks.
> Yes please.
>
> Also, Matt, does it/would it work to stop the memory fragmentation issue
> if we push the allocation up to the next page size and just waste the
> 3k bytes?  Some people might be willing to make that trade off to get
> long up times and full 9k jumbo's.
>
>
> Thanks,
> Rod
>
> > -M
> >
> > On Tue, Mar 5, 2019 at 11:13 AM Eric Joyner <erj at freebsd.org> wrote:
> > >
> > > Author: erj
> > > Date: Tue Mar  5 19:12:51 2019
> > > New Revision: 344817
> > > URL: https://svnweb.freebsd.org/changeset/base/344817
> > >
> > > Log:
> > >   Remove references to CONTIGMALLOC_WORKS in iflib and em
> > >
> > >   From Jake:
> > >   "The iflib_fl_setup() function tries to pick various buffer sizes
> based
> > >   on the max_frame_size value defined by the parent driver. However,
> this
> > >   code was wrapped under CONTIGMALLOC_WORKS, which was never actually
> > >   defined anywhere.
> > >
> > >   This same code pattern was used in if_em.c, likely trying to match
> > >   what iflib uses.
> > >
> > >   Since CONTIGMALLOC_WORKS is not defined, remove this dead code from
> > >   iflib_fl_setup and if_em.c
> > >
> > >   Given that various iflib drivers appear to be using a similar
> > >   calculation, it might be worth making this buffer size a value that
> the
> > >   driver can peek at in the future."
> > >
> > >   Submitted by: Jacob Keller <jacob.e.keller at intel.com>
> > >   Reviewed by:  shurd@
> > >   MFC after:    1 week
> > >   Sponsored by: Intel Corporation
> > >   Differential Revision:        https://reviews.freebsd.org/D19199
> > >
> > > Modified:
> > >   head/sys/dev/e1000/if_em.c
> > >   head/sys/net/iflib.c
> > >
> > > Modified: head/sys/dev/e1000/if_em.c
> > >
> ==============================================================================
> > > --- head/sys/dev/e1000/if_em.c  Tue Mar  5 19:08:37 2019
> (r344816)
> > > +++ head/sys/dev/e1000/if_em.c  Tue Mar  5 19:12:51 2019
> (r344817)
> > > @@ -1276,15 +1276,8 @@ em_if_init(if_ctx_t ctx)
> > >          */
> > >         if (adapter->hw.mac.max_frame_size <= 2048)
> > >                 adapter->rx_mbuf_sz = MCLBYTES;
> > > -#ifndef CONTIGMALLOC_WORKS
> > >         else
> > >                 adapter->rx_mbuf_sz = MJUMPAGESIZE;
> > > -#else
> > > -       else if (adapter->hw.mac.max_frame_size <= 4096)
> > > -               adapter->rx_mbuf_sz = MJUMPAGESIZE;
> > > -       else
> > > -               adapter->rx_mbuf_sz = MJUM9BYTES;
> > > -#endif
> > >         em_initialize_receive_unit(ctx);
> > >
> > >         /* Use real VLAN Filter support? */
> > >
> > > Modified: head/sys/net/iflib.c
> > >
> ==============================================================================
> > > --- head/sys/net/iflib.c        Tue Mar  5 19:08:37 2019
> (r344816)
> > > +++ head/sys/net/iflib.c        Tue Mar  5 19:12:51 2019
> (r344817)
> > > @@ -2187,17 +2187,8 @@ iflib_fl_setup(iflib_fl_t fl)
> > >          */
> > >         if (sctx->isc_max_frame_size <= 2048)
> > >                 fl->ifl_buf_size = MCLBYTES;
> > > -#ifndef CONTIGMALLOC_WORKS
> > >         else
> > >                 fl->ifl_buf_size = MJUMPAGESIZE;
> > > -#else
> > > -       else if (sctx->isc_max_frame_size <= 4096)
> > > -               fl->ifl_buf_size = MJUMPAGESIZE;
> > > -       else if (sctx->isc_max_frame_size <= 9216)
> > > -               fl->ifl_buf_size = MJUM9BYTES;
> > > -       else
> > > -               fl->ifl_buf_size = MJUM16BYTES;
> > > -#endif
> > >         if (fl->ifl_buf_size > ctx->ifc_max_fl_buf_size)
> > >                 ctx->ifc_max_fl_buf_size = fl->ifl_buf_size;
> > >         fl->ifl_cltype = m_gettype(fl->ifl_buf_size);
> > >
> >
> >
>
> --
> Rod Grimes
> rgrimes at freebsd.org
>
>