Date: Tue, 5 Mar 2019 13:06:18 -0800 From: Matthew Macy <mat.macy@gmail.com> To: Eric Joyner <erj@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r344817 - in head/sys: dev/e1000 net Message-ID: <CAPrugNoO4YgHZ2T6e0ZYbOLb1B_K0CwDsN3AUDTXC%2BGViYV6fw@mail.gmail.com> In-Reply-To: <201903051912.x25JCqOj068140@repo.freebsd.org> References: <201903051912.x25JCqOj068140@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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. -M On Tue, Mar 5, 2019 at 11:13 AM Eric Joyner <erj@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@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); >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPrugNoO4YgHZ2T6e0ZYbOLb1B_K0CwDsN3AUDTXC%2BGViYV6fw>