From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id A4A6C689B for ; Wed, 11 Jan 2017 18:47:45 +0100 (CET) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga103.jf.intel.com with ESMTP; 11 Jan 2017 09:47:44 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,346,1477983600"; d="scan'208";a="47885035" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.38]) ([10.237.220.38]) by orsmga004.jf.intel.com with ESMTP; 11 Jan 2017 09:47:43 -0800 To: "Ananyev, Konstantin" , Stephen Hemminger References: <1483048216-2936-1-git-send-email-s.vyazmitinov@brain4net.com> <20170111081759.7b1ee146@xeon-e3> <2601191342CEEE43887BDE71AB9772583F103F8F@irsmsx105.ger.corp.intel.com> <20170111093559.753a0fc9@xeon-e3> <2601191342CEEE43887BDE71AB9772583F103FCA@irsmsx105.ger.corp.intel.com> Cc: Sergey Vyazmitinov , "olivier.matz@6wind.com" , "dev@dpdk.org" From: Ferruh Yigit Message-ID: <63819ae1-f056-0ad7-b7dd-041fe1fe08fa@intel.com> Date: Wed, 11 Jan 2017 17:47:42 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <2601191342CEEE43887BDE71AB9772583F103FCA@irsmsx105.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs 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: Wed, 11 Jan 2017 17:47:46 -0000 On 1/11/2017 5:43 PM, Ananyev, Konstantin wrote: > > >> -----Original Message----- >> From: Stephen Hemminger [mailto:stephen@networkplumber.org] >> Sent: Wednesday, January 11, 2017 5:36 PM >> To: Ananyev, Konstantin >> Cc: Sergey Vyazmitinov ; olivier.matz@6wind.com; Yigit, Ferruh ; dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs >> >> On Wed, 11 Jan 2017 17:28:21 +0000 >> "Ananyev, Konstantin" wrote: >> >>>> -----Original Message----- >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger >>>> Sent: Wednesday, January 11, 2017 4:18 PM >>>> To: Sergey Vyazmitinov >>>> Cc: olivier.matz@6wind.com; Yigit, Ferruh ; dev@dpdk.org >>>> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs >>>> >>>> On Fri, 30 Dec 2016 04:50:16 +0700 >>>> Sergey Vyazmitinov wrote: >>>> >>>>> /** >>>>> + * Free n packets mbuf back into its original mempool. >>>>> + * >>>>> + * Free each mbuf, and all its segments in case of chained buffers. Each >>>>> + * segment is added back into its original mempool. >>>>> + * >>>>> + * @param mp >>>>> + * The packets mempool. >>>>> + * @param mbufs >>>>> + * The packets mbufs array to be freed. >>>>> + * @param n >>>>> + * Number of packets. >>>>> + */ >>>>> +static inline void rte_pktmbuf_free_bulk(struct rte_mempool *mp, >>>>> + struct rte_mbuf **mbufs, unsigned n) >>>>> +{ >>>>> + struct rte_mbuf *mbuf, *m_next; >>>>> + unsigned i; >>>>> + for (i = 0; i < n; ++i) { >>>>> + mbuf = mbufs[i]; >>>>> + __rte_mbuf_sanity_check(mbuf, 1); >>>>> + >>>>> + mbuf = mbuf->next; >>>>> + while (mbuf != NULL) { >>>>> + m_next = mbuf->next; >>>>> + rte_pktmbuf_free_seg(mbuf); >>>>> + mbuf = m_next; >>>>> + } >>>>> + } >>>>> + rte_mempool_put_bulk(mp, (void * const *)mbufs, n); >>>>> +} >>>> >>>> The mbufs may come from different pools. You need to handle that. >>> >>> I suppose both stituations are possible: >>> 1) user knows off-hand that all mbufs in the group are from the same mempool >>> 2) user can't guarantee that all mbufs in the group are from same mempool. >>> >>> As I understand that patch is for case 1) only. >>> For 2) it could be a separate function and separate patch. >>> >>> Konstantin >>> >>> >> >> Please don't make unnecessary assumptions in pursuit of minor optimizations. > > I don't suggest to make *any* assumptions. > What I am saying we can have 2 functions for two different cases. kni_free_mbufs() is static function. Even user knows if all mbufs are some same pool or not, can't pass this information to the free function. Of course this information can be passed via new API, or as an update to exiting API, but I think it is better to update free function to cover both cases instead of getting this information from user. > Obviously we'll have to document it properly. > Konstantin > >> It is trivial to write a correct free bulk that handles pool changing. >> Also the free_seg could be bulked as well.