From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 82DDC568F for ; Thu, 19 Mar 2015 11:06:45 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga101.jf.intel.com with ESMTP; 19 Mar 2015 03:06:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,428,1422950400"; d="scan'208";a="694431515" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by fmsmga002.fm.intel.com with ESMTP; 19 Mar 2015 03:06:40 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.117]) by irsmsx110.ger.corp.intel.com ([169.254.15.236]) with mapi id 14.03.0195.001; Thu, 19 Mar 2015 10:06:39 +0000 From: "Ananyev, Konstantin" To: Olivier MATZ , Neil Horman , "vadim.suraev@gmail.com" Thread-Topic: [dpdk-dev] [PATCH v2] rte_mbuf: mbuf bulk alloc/free functions added + unittest Thread-Index: AQHQYbk48Ca1Y5Q95kWd76b21g7cN50iuT8AgADERoCAABZHUA== Date: Thu, 19 Mar 2015 10:06:38 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258213F7443@irsmsx105.ger.corp.intel.com> References: <1426710078-11230-1-git-send-email-vadim.suraev@gmail.com> <20150318205856.GA5151@neilslaptop.think-freely.org> <550A8BB5.9040104@6wind.com> In-Reply-To: <550A8BB5.9040104@6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2] rte_mbuf: mbuf bulk alloc/free functions added + unittest 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: Thu, 19 Mar 2015 10:06:46 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ > Sent: Thursday, March 19, 2015 8:41 AM > To: Neil Horman; vadim.suraev@gmail.com > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2] rte_mbuf: mbuf bulk alloc/free functio= ns added + unittest >=20 > Hi Neil, >=20 > On 03/18/2015 09:58 PM, Neil Horman wrote: > >> +/** > >> + * Free a bulk of mbufs into its original mempool. > >> + * This function assumes: > >> + * - refcnt equals 1 > >> + * - mbufs are direct > >> + * - all mbufs must belong to the same mempool > >> + * > >> + * @param mbufs > >> + * Array of pointers to mbuf > >> + * @param count > >> + * Array size > >> + */ > >> +static inline void rte_pktmbuf_bulk_free(struct rte_mbuf **mbufs, > >> + unsigned count) > >> +{ > >> + unsigned idx; > >> + > >> + RTE_MBUF_ASSERT(count > 0); > >> + > >> + for (idx =3D 0; idx < count; idx++) { > >> + RTE_MBUF_ASSERT(mbufs[idx]->pool =3D=3D mbufs[0]->pool); > >> + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) =3D=3D 1); > >> + rte_mbuf_refcnt_set(mbufs[idx], 0); > > This is really a misuse of the API. The entire point of reference coun= ting is > > to know when an mbuf has no more references and can be freed. By forci= ng all > > the reference counts to zero here, you allow the refcnt infrastructure = to be > > circumvented, causing memory leaks. > > > > I think what you need to do here is enhance the underlying pktmbuf inte= rface > > such that an rte_mbuf structure has a destructor method association wit= h it > > which is called when its refcnt reaches zero. That way the > > rte_pktmbuf_bulk_free function can just decrement the refcnt on each > > mbuf_structure, and the pool as a whole can be returned when the destru= ctor > > function discovers that all mbufs in that bulk pool are freed. >=20 > I don't really understand what's the problem here. The API explicitly > describes the conditions for calling this functions: the segments are > directs, they belong to the same mempool, and their refcnt is 1. >=20 > This function could be useful in a driver which knows that the mbuf > it allocated matches this conditions. I think an application that > only uses direct mbufs and one mempool could also use this function. I also don't see anything wrong with that function. As long, as user makes sure that all mbufs in the bulk satisfy the required= conditions it should be ok, I think. Of course, it's usage is limited, but I suppose the author has some scenari= o in mind, when introduced it. Konstantin >=20 > Regards, > Olivier