From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48]) by dpdk.org (Postfix) with ESMTP id B150F2B86 for ; Tue, 28 Feb 2017 15:51:13 +0100 (CET) Received: by mail-wm0-f48.google.com with SMTP id v186so87424259wmd.0 for ; Tue, 28 Feb 2017 06:51:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=tss4OoM/7scWFuAlCf1MYONjtJ8TLNWOnrMcrlvRceg=; b=AYk7a2bfo7jSrSWvZgyKHhnQASJIZ1+Q8FjGbYHX/dLoVFTS/h56RteOFeLJsBqTJJ Q2Bzpt7nitQvyYLdx0okWiheDRYXhBBLuFFp/tbNZLXliC8sDLk+QXnAc6/Vb9eyBEsQ QeJkCkjzXAKSY8u60bbi6QUvUAzbdvFk4ZC9kbsaOEsDnWVxFeuyQAsMUWn97igpyL3L Zx/F5WzH2aKGXXAchkGhKsv9KJ712Gag++0WPg1oAUcw4pDfRmPEJPveZgkPCeQBfaNq vY71Qfm8LvLhWirIsu354hfyP9v5pYmv8Gfiw5kjPtXaAhuBDYhLCzYETjdMNv2HEV+2 2L3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=tss4OoM/7scWFuAlCf1MYONjtJ8TLNWOnrMcrlvRceg=; b=lQp31/TFvlhGcGg+o8NUcSJ9oh1L6xbKwyZl6nS9bG8CgK2mGevDzC8oBFvj+YdoFd JNhlLgfgHlF7oC4KGKNpgek3dp/z+OR/U+5pfSpiyw5gnniVql68pi8ix4sdL2bUUM4r bMSzURf7bsKOT9JvJTCFuWfwhZJQH3C4P75FuQ0VxVsZMU/ufOEzx458LO6htzy0X6Wy ZlHhqlzrmKFneHeJHK6ojpyrAktQVIAaw9KjbA2Cpf/WQj40iAnQOJdjGS974x6e1Qz8 TASnyFYBxdNUskWoiW+fsn1RzJhOQHacR8GrJgkhC26r9JA3X6Z+q/8ZuAuxTxgyO5qC ck0w== X-Gm-Message-State: AMke39mfk3eJ/j8wOZ7oY7gEUhspgNvjNu5VQPSO08102s1DA0AGRUmYywojn9OyDny0GfHg X-Received: by 10.28.7.13 with SMTP id 13mr11390380wmh.16.1488293473436; Tue, 28 Feb 2017 06:51:13 -0800 (PST) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id o52sm2647131wrb.51.2017.02.28.06.51.13 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 28 Feb 2017 06:51:13 -0800 (PST) Date: Tue, 28 Feb 2017 15:51:11 +0100 From: Olivier Matz To: Bruce Richardson Cc: dev@dpdk.org Message-ID: <20170228155111.2dbee114@platinum> In-Reply-To: <20170124155049.GA172024@bricha3-MOBL3.ger.corp.intel.com> References: <1485271173-13408-1-git-send-email-olivier.matz@6wind.com> <1485271173-13408-4-git-send-email-olivier.matz@6wind.com> <20170124155049.GA172024@bricha3-MOBL3.ger.corp.intel.com> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC 3/8] mbuf: set mbuf fields while in pool X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Feb 2017 14:51:13 -0000 Hi Bruce, On Tue, 24 Jan 2017 15:50:49 +0000, Bruce Richardson wrote: > On Tue, Jan 24, 2017 at 04:19:28PM +0100, Olivier Matz wrote: > > Set the value of m->refcnt to 1, m->nb_segs to 1 and m->next > > to NULL when the mbuf is stored inside the mempool (unused). > > This is done in rte_pktmbuf_prefree_seg(), before freeing or > > recycling a mbuf. > > > > Before this patch, the value of m->refcnt was expected to be 0 > > while in pool. > > > > The objectives are: > > > > - to avoid drivers to set m->next to NULL in the early Rx path, > > since this field is in the second 64B of the mbuf and its access > > could trigger a cache miss > > > > - rationalize the behavior of raw_alloc/raw_free: one is now the > > symmetric of the other, and refcnt is never changed in these > > functions. > > > > Signed-off-by: Olivier Matz > > --- > > drivers/net/mlx5/mlx5_rxtx.c | 5 ++--- > > drivers/net/mpipe/mpipe_tilegx.c | 1 + > > lib/librte_mbuf/rte_mbuf.c | 2 ++ > > lib/librte_mbuf/rte_mbuf.h | 45 > > +++++++++++++++++++++++++++++----------- 4 files changed, 38 > > insertions(+), 15 deletions(-) > > > /* compat with older versions */ > > __rte_deprecated > > -static inline void __attribute__((always_inline)) > > +static inline void > > __rte_mbuf_raw_free(struct rte_mbuf *m) > > { > > rte_mbuf_raw_free(m); > > @@ -1218,8 +1232,12 @@ static inline void rte_pktmbuf_detach(struct > > rte_mbuf *m) m->data_len = 0; > > m->ol_flags = 0; > > > > - if (rte_mbuf_refcnt_update(md, -1) == 0) > > + if (rte_mbuf_refcnt_update(md, -1) == 0) { > > Minor nit, but in the case that we only have a single reference to the > mbufs, we are always setting that to zero just to re-increment it to 1 > again. > > > + md->next = NULL; > > + md->nb_segs = 1; > > + rte_mbuf_refcnt_set(md, 1); > > rte_mbuf_raw_free(md); > > + } > > } > > > > /** I'm trying to gather the comments that have been made on this patchset. About this one, I think it would be more complex to change the code to avoid to set the refcnt twice: - we would need to duplicate code from rte_mbuf_refcnt_update(), which I think is not a very good idea, due to the big comment - it would make the detach code less readable - it's even not sure that it will be more performant: since rte_mbuf_refcnt_update() is inline, the compiler is probably able to do the simplification by itself. Olivier