From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Hanoch Haim (hhaim)" Subject: Re: [PATCH v2] mbuf: optimize rte_mbuf_refcnt_update Date: Sun, 27 Dec 2015 09:39:27 +0000 Message-ID: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , "Hanoch Haim \(hhaim\)" , "Ido Barnea \(ibarnea\)" , "Itay Marom \(imarom\)" To: "bruce.richardson@intel.com" Return-path: Received: from alln-iport-4.cisco.com (alln-iport-4.cisco.com [173.37.142.91]) by dpdk.org (Postfix) with ESMTP id 666B85A49 for ; Sun, 27 Dec 2015 10:39:30 +0100 (CET) Content-Language: en-US List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Bruce, I'm Hanoch from Cisco Systems works for the https://github.com/cisco-syste= m-traffic-generator/trex-core traffic generator project. While upgrading from DPDK 1.8 to 2.2 Ido found that the following commit cr= eates a mbuf corruption and result in Tx hang commit f20b50b946da9070d21e392e4dbc7d9f68bc983e Author: Olivier Matz Date: Mon Jun 8 16:57:22 2015 +0200 Looking at the change it is clear why there is an issue, wanted to get your= input. Init ----- alloc const mbuf =3D=3D> mbuf-a (ref=3D1) Simple case that works --------------------- thread 1 , tx: alloc-mbuf->attach(mbuf-a) (ref=3D2) inc- non atomic thread 1 , tx: alloc-mbuf->attach(mbuf-a) (ref32) inc- atomic thread 1 , drv : free() (ref=3D2) dec- atomic thread 1 , drv : free() (ref=3D3) dec - non atomic Simple case that does not work --------------------- Both do that in parallel thread 2 tx : alloc-mbuf->attach(mbuf-a) (ref=3D2) inc- non atomic thread 1 tx : alloc-mbuf->attach(mbuf-a) (ref=3D2) inc- non atomic =3D=3D> ref in corrupted thread 1 drv : free() (ref=3D0) - free thread 1 drv : free() (ref=3D-1) - ??? Just as a side note that in our case we could benefit from this optimizatio= n by making a small change in our code, as our common case could be alloc/f= ree of simple mbuf and in such scenario there is no atomic operation in bot= h cases. But I think the general case is broken. Could you explain what was your use case or this optimization? Is it a vali= d example the aforementioned example thanks, Hanoh Cisco Systems On Mon, Jun 08, 2015 at 04:57:22PM +0200, Olivier Matz wrote: > In __rte_pktmbuf_prefree_seg(), there was an optimization to avoid using > a costly atomic operation when updating the mbuf reference counter if > its value is 1. Indeed, it means that we are the only owner of the mbuf, > and therefore nobody can change it at the same time. > > We can generalize this optimization directly in rte_mbuf_refcnt_update() > so the other callers of this function, like rte_pktmbuf_attach(), can > also take advantage of this optimization. > > Signed-off-by: Olivier Matz > Acked-by: Bruce Richardson > Hanoh