From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nk11p03mm-asmtp002.mac.com (nk11p03mm-asmtpout002.mac.com [17.158.232.237]) by dpdk.org (Postfix) with ESMTP id 74EFFC39E for ; Mon, 3 Aug 2015 09:09:36 +0200 (CEST) Received: from [192.168.31.134] (250.229.126.124.broad.bjtelecom.net [124.126.229.250]) by nk11p03mm-asmtp002.mac.com (Oracle Communications Messaging Server 7.0.5.35.0 64bit (built Mar 31 2015)) with ESMTPSA id <0NSH00H9FVVDS840@nk11p03mm-asmtp002.mac.com> for dev@dpdk.org; Mon, 03 Aug 2015 07:09:17 +0000 (GMT) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151,1.0.33,0.0.0000 definitions=2015-08-03_01:2015-08-01,2015-08-02,1970-01-01 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=2 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=7.0.1-1412110000 definitions=main-1508030123 Content-type: text/plain; charset=gb2312 MIME-version: 1.0 (Mac OS X Mail 8.2 \(2098\)) From: HePeng In-reply-to: <6A0DE07E22DDAD4C9103DF62FEBC0909D1A59D@shsmsx102.ccr.corp.intel.com> Date: Mon, 03 Aug 2015 15:09:11 +0800 Content-transfer-encoding: quoted-printable Message-id: <470BDF91-6FAE-4605-885E-78F02F0F0FBF@icloud.com> References: <1438392394-19653-1-git-send-email-xnhp0320@icloud.com> <6A0DE07E22DDAD4C9103DF62FEBC0909D1A508@shsmsx102.ccr.corp.intel.com> <179B898C-F03A-4250-8857-236CFC5274BA@icloud.com> <6A0DE07E22DDAD4C9103DF62FEBC0909D1A59D@shsmsx102.ccr.corp.intel.com> To: "Lu, Wenzhuo" X-Mailer: Apple Mail (2.2098) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] [new]ixgbe:set txep.mbuf to NULL when calling ixgbe_tx_free_bufs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Aug 2015 07:09:37 -0000 Hi Wenzhuo, The issue is in the function *ixgbe_dev_free_queues* called in = the *ixgbe_dev_close*.=20 The *ixgbe_dev_free_queues* will call = *ixgbe_rx_queue_release_mbuf* to recycle all the mbuf on=20 the queues. If some mbufs have already been recycled by the = *ixgbe_tx_free_bufs*, their=20 ref cnt is 0.=20 However, since the pointers are not set to NULL, = *ixgbe_rx_queue_release_mbuf* will also check the mbufs whose ref cnt=20 is 0, then if one enables *CONFIG_RTE_MBUF_DEBUG*, the sanity checks = will warn that the ref cnt is bad, and the=20 program will bail out. As you said if this is a designed behavior, you need to fix the = code in *ixgbe_rx_queue_release_mbuf* to skip the=20 mbuf that already been recycled.=20 > =D4=DA 2015=C4=EA8=D4=C23=C8=D5=A3=AC=CF=C2=CE=E71:16=A3=ACLu, Wenzhuo = =D0=B4=B5=C0=A3=BA >=20 > Hi Peng, > Would you like to tell me more details about the panic? > I saw there's refcnt check in rte_mbuf_sanity_check. I'm not sure what = sanity check you want to add. > Thanks. >=20 >> -----Original Message----- >> From: HePeng [mailto:xnhp0320@icloud.com] >> Sent: Monday, August 3, 2015 10:54 AM >> To: Lu, Wenzhuo >> Subject: Re: [dpdk-dev] [PATCH] [new]ixgbe:set txep.mbuf to NULL when = calling >> ixgbe_tx_free_bufs >>=20 >> Hi, Wenzhuo >> It will cause panic if you enable *CONFIG_RTE_MBUF_DEBUG* in you >> config file. So if it is a designed behavior, some code fix may need = for >> *mbuf_sanity_check*. >> Thanks. >>=20 >>=20 >>> =D4=DA 2015=C4=EA8=D4=C23=C8=D5=A3=AC=C9=CF=CE=E710:46=A3=ACLu, = Wenzhuo =D0=B4 >> =B5=C0=A3=BA >>>=20 >>> Hi Peng, >>>=20 >>>> -----Original Message----- >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of hepeng >>>> Sent: Saturday, August 1, 2015 9:27 AM >>>> To: dev@dpdk.org >>>> Subject: [dpdk-dev] [PATCH] [new]ixgbe:set txep.mbuf to NULL when = calling >>>> ixgbe_tx_free_bufs >>>>=20 >>>> In *ixgbe_tx_free_bufs*, after recycling some tx entries, one = should set their >>>> mbuf pointers to NULL. >>>>=20 >>>> The first path is not correct, the txep->mbuf should be set to NULL = no matter >> if >>>> it is recycled into mempool >>>> Signed-off-by: hepeng >>>> --- >>>> drivers/net/ixgbe/ixgbe_rxtx_vec.c | 13 ++++++++++++- >>>> 1 file changed, 12 insertions(+), 1 deletion(-) >>>>=20 >>>> diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c >>>> b/drivers/net/ixgbe/ixgbe_rxtx_vec.c >>>> index 1c16dec..e7ce740 100644 >>>> --- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c >>>> +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c >>>> @@ -612,6 +612,7 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq) >>>> */ >>>> txep =3D &txq->sw_ring_v[txq->tx_next_dd - (n - 1)]; >>>> m =3D __rte_pktmbuf_prefree_seg(txep[0].mbuf); >>>> + txep[0].mbuf =3D NULL; >>>> if (likely(m !=3D NULL)) { >>>> free[0] =3D m; >>>> nb_free =3D 1; >>>> @@ -632,11 +633,21 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue = *txq) >>>> } else { >>>> for (i =3D 1; i < n; i++) { >>>> m =3D __rte_pktmbuf_prefree_seg(txep[i].mbuf); >>>> - if (m !=3D NULL) >>>> + if (m !=3D NULL) { >>>> rte_mempool_put(m->pool, m); >>>> + } >>>> } >>>> } >>>>=20 >>>> + /* >>>> + * No matter the mbufs have been put back to mempool or not, >>>> + * we should set the txep[i].mbuf to NULL >>>> + */ >>>> + >>>> + for( i =3D 1; i < n; i++) { >>>> + txep[i].mbuf =3D NULL; >>>> + } >>>> + >>>> /* buffers were freed, update counters */ >>>> txq->nb_tx_free =3D (uint16_t)(txq->nb_tx_free + = txq->tx_rs_thresh); >>>> txq->tx_next_dd =3D (uint16_t)(txq->tx_next_dd + = txq->tx_rs_thresh); >>>> -- >>>> 1.9.1 >>>=20 >>> NACK. >>> Thanks for looking into this code. But it's designed behavior, not = an issue. >>> BTW, if you want to send a new version, the tittle should be like = this [PATCH v2] >> ixgbe: ..., and add "--in-reply-to your original mail" when sending = the mail, and >> add a v2 comments. You can reference the other's v2 patches for = detail. >>>=20 >=20