From owner-svn-src-head@freebsd.org Tue Mar 5 21:06:32 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id ED74C1513530; Tue, 5 Mar 2019 21:06:31 +0000 (UTC) (envelope-from mat.macy@gmail.com) Received: from mail-io1-xd35.google.com (mail-io1-xd35.google.com [IPv6:2607:f8b0:4864:20::d35]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 75881889B2; Tue, 5 Mar 2019 21:06:31 +0000 (UTC) (envelope-from mat.macy@gmail.com) Received: by mail-io1-xd35.google.com with SMTP id p196so8266790iod.9; Tue, 05 Mar 2019 13:06:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Z2mUZGK5VQuBJPk4RWGrJtB5EYA5oi9Dy8iNeVGZ1Ho=; b=GTh5f+ZfrQy5MTD/RvBRL17kXhhv5ewWN+RoxcN3n4p+d9g+l7jyqzqlLMmC8BOEl8 5qc8yqZVKoZsOhisdyb+aYulUd7VaEuA/IPgxJF6FAQ3munnU68ys4n8JhqkuCe1okto yFr8YtSbCqTTB/UI3rQwaIfYv01A32LdV3MFW4qSRgM1TZDQemOm+5sILTw6yv+fuOQN sDelu9PRgbcy4IrzqVGqUKnnv9rJ8I9gLLma4mb7wvz6h6er08lVmO9xW/TGWEfU8Zy0 w2p1J3xrrwDpDSUf8loYR5x1BJhqkeWdGeKoQ+I0m5vdn3ML+bwqSIxmkeGCTUqwdvLC qgEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Z2mUZGK5VQuBJPk4RWGrJtB5EYA5oi9Dy8iNeVGZ1Ho=; b=nPqqux24bbUUIrZxwfyXSz3llh+YOEpN8UjK6fgFixi069oLpH4hY2zB3fEAssgnsc PP3vQYOiJ1D7L9vDkWUax2JdYWyMdlHPXNmSuBdSnhkvObPUqVsGZnn0+rhegVVVawW5 QqV8atGOaDSILA/gcar5yroJB6WkGZ1ZXWOBlA38Wgo3xhy5DwAjNEfWPRm8aIs9FPFP 5Zggzas5WEgiw+sBKTpzPJrwwGk/ycQAmO4P56SaPDNS9253xRj3rJctWzQhw6FvqPpc GMPU/pdzZ3JEao9dOFTQ42IpzlwxCYYq2cXzc29bU9No8sno+qF9hMLfL5RbPaLG5oWt cjIQ== X-Gm-Message-State: APjAAAUBSeHdKUg4KFoBTRk/1Zm3XH8yW9FHpTnjb7Of/9Sl+6bkIdT+ M3CQC/MLw2NZkNuEIVkwKh8UbwpcA7yIzx5f/GdmTOC8 X-Google-Smtp-Source: APXvYqz0T8JhsrBk8+a0Wl8uCyFZMB9pKCgK/Frhgc29+NuqhJ3aPhF0trd5XzH5ltnG2x8WxH7juGHBUahGpN0iwGM= X-Received: by 2002:a6b:7d47:: with SMTP id d7mr1193356ioq.237.1551819990102; Tue, 05 Mar 2019 13:06:30 -0800 (PST) MIME-Version: 1.0 References: <201903051912.x25JCqOj068140@repo.freebsd.org> In-Reply-To: <201903051912.x25JCqOj068140@repo.freebsd.org> From: Matthew Macy Date: Tue, 5 Mar 2019 13:06:18 -0800 Message-ID: Subject: Re: svn commit: r344817 - in head/sys: dev/e1000 net To: Eric Joyner Cc: src-committers , svn-src-all , svn-src-head Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 75881889B2 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.99 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.99)[-0.988,0]; REPLY(-4.00)[]; TAGGED_FROM(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2019 21:06:32 -0000 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 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 > 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); >