Date: Tue, 5 Mar 2019 13:12:29 -0800 (PST) From: "Rodney W. Grimes" <freebsd@gndrsh.dnsmgr.net> To: Matthew Macy <mat.macy@gmail.com> Cc: Eric Joyner <erj@freebsd.org>, 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: <201903052112.x25LCToe082646@ gndrsh.dnsmgr.net> In-Reply-To: <CAPrugNoO4YgHZ2T6e0ZYbOLb1B_K0CwDsN3AUDTXC%2BGViYV6fw@mail.gmail.com>
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. 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@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); > > > > -- Rod Grimes rgrimes@freebsd.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201903052112.x25LCToe082646>