From owner-svn-src-all@freebsd.org Wed Aug 16 20:54:47 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A541DDD0CF3; Wed, 16 Aug 2017 20:54:47 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (glebi.us [96.95.210.25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "cell.glebi.us", Issuer "cell.glebi.us" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 73EA46FA07; Wed, 16 Aug 2017 20:54:46 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (localhost [127.0.0.1]) by cell.glebi.us (8.15.2/8.15.2) with ESMTPS id v7GKskR7017466 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 16 Aug 2017 13:54:46 -0700 (PDT) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebi.us (8.15.2/8.15.2/Submit) id v7GKskQ7017465; Wed, 16 Aug 2017 13:54:46 -0700 (PDT) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebi.us: glebius set sender to glebius@FreeBSD.org using -f Date: Wed, 16 Aug 2017 13:54:46 -0700 From: Gleb Smirnoff To: Sean Bruno , Matt Macy Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r322338 - head/sys/net Message-ID: <20170816205446.GL1113@FreeBSD.org> References: <201708100343.v7A3hNwR068837@repo.freebsd.org> <20170816204327.GK1113@FreeBSD.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="0OAP2g/MAC+5xKAE" Content-Disposition: inline In-Reply-To: <20170816204327.GK1113@FreeBSD.org> User-Agent: Mutt/1.8.3 (2017-05-23) X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Aug 2017 20:54:47 -0000 --0OAP2g/MAC+5xKAE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Aug 16, 2017 at 01:43:27PM -0700, Gleb Smirnoff wrote: T> S> MPASS(m->m_nextpkt == NULL); T> S> - T> S> - m_free(m); T> S> + /* if the number of clusters exceeds the number of segments T> S> + * there won't be space on the ring to save a pointer to each T> S> + * cluster so we simply free the list here T> S> + */ T> S> + if (m->m_flags & M_TOOBIG) { T> S> + m_freem(m); T> S> + } else { T> S> + m_free(m); T> S> + } T> S> ifsd_m[cidx] = NULL; T> S> #if MEMORY_LOGGING T> S> txq->ift_dequeued++; T> T> Can you please explain the goal of the change? AFAIK, the problem T> could be fixed with one liner: T> T> - m_free(m); T> + m_freem(m); T> T> n the iflib_tx_desc_free(). I'm probably wrong on one liner, but still I don't see reason to have flag. Since we clear m_next in normal case, doing m_freem() always is fine. Suggested patch attached. -- Totus tuus, Glebius. --0OAP2g/MAC+5xKAE Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="iflib.c.diff" Index: sys/net/iflib.c =================================================================== --- sys/net/iflib.c (revision 322587) +++ sys/net/iflib.c (working copy) @@ -267,8 +267,6 @@ iflib_get_sctx(if_ctx_t ctx) #define RX_SW_DESC_INUSE (1 << 3) #define TX_SW_DESC_MAPPED (1 << 4) -#define M_TOOBIG M_UNUSED_8 - typedef struct iflib_sw_rx_desc_array { bus_dmamap_t *ifsd_map; /* bus_dma maps for packet */ struct mbuf **ifsd_m; /* pkthdr mbufs */ @@ -2934,7 +2932,6 @@ iflib_busdma_load_mbuf_sg(iflib_txq_t txq, bus_dma } while (m != NULL); if (count > *nsegs) { ifsd_m[pidx] = *m0; - ifsd_m[pidx]->m_flags |= M_TOOBIG; return (0); } m = *m0; @@ -3246,15 +3243,7 @@ iflib_tx_desc_free(iflib_txq_t txq, int n) if ((m = ifsd_m[cidx]) != NULL) { /* XXX we don't support any drivers that batch packets yet */ MPASS(m->m_nextpkt == NULL); - /* if the number of clusters exceeds the number of segments - * there won't be space on the ring to save a pointer to each - * cluster so we simply free the list here - */ - if (m->m_flags & M_TOOBIG) { - m_freem(m); - } else { - m_free(m); - } + m_freem(m); ifsd_m[cidx] = NULL; #if MEMORY_LOGGING txq->ift_dequeued++; --0OAP2g/MAC+5xKAE--