Date: Tue, 5 Mar 2019 14:05:48 -0800 From: Eric Joyner <erj@freebsd.org> To: rgrimes@freebsd.org, Matthew Macy <mat.macy@gmail.com> Cc: src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org>, Jacob Keller <jacob.e.keller@intel.com> Subject: Re: svn commit: r344817 - in head/sys: dev/e1000 net Message-ID: <CAKdFRZjes=VXo35kc2bHU=G_k72FREv3TC1V1QChTRKr2wuiDA@mail.gmail.com> In-Reply-To: <5c7ee64f.1c69fb81.1168f.5802SMTPIN_ADDED_BROKEN@mx.google.com> References: <CAPrugNoO4YgHZ2T6e0ZYbOLb1B_K0CwDsN3AUDTXC%2BGViYV6fw@mail.gmail.com> <5c7ee64f.1c69fb81.1168f.5802SMTPIN_ADDED_BROKEN@mx.google.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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@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?CAKdFRZjes=VXo35kc2bHU=G_k72FREv3TC1V1QChTRKr2wuiDA>