From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 27B441B14C for ; Wed, 26 Sep 2018 12:41:21 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Sep 2018 03:41:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,306,1534834800"; d="scan'208";a="260365560" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.39]) ([10.237.221.39]) by orsmga005.jf.intel.com with ESMTP; 26 Sep 2018 03:41:20 -0700 To: Igor Ryzhov , dev@dpdk.org References: <20180802142522.57900-1-iryzhov@nfware.com> <20180923191202.64896-1-iryzhov@nfware.com> From: Ferruh Yigit Openpgp: preference=signencrypt Message-ID: <594e0f5a-7f4f-5713-3cdf-d41e492e0a04@intel.com> Date: Wed, 26 Sep 2018 11:41:19 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180923191202.64896-1-iryzhov@nfware.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] kni: dynamically allocate memory for each KNI 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, 26 Sep 2018 10:41:22 -0000 On 9/23/2018 8:12 PM, Igor Ryzhov wrote: > Long time ago preallocation of memory for KNI was introduced in commit > 0c6bc8e. It was done because of lack of ability to free previously > allocated memzones, which led to memzone exhaustion. Currently memzones > can be freed and this patch uses this ability for dynamic KNI memory > allocation. Hi Igor, Good cleanup, thanks. +1 to eal_tailq for ctx A few minor comments below, but they are not significant enough to block the patch, please let us know if you don't have bandwidth for a new version. > Signed-off-by: Igor Ryzhov <...> > @@ -294,41 +180,52 @@ rte_kni_alloc(struct rte_mempool *pktmbuf_pool, > { > int ret; > struct rte_kni_device_info dev_info; > - struct rte_kni *ctx; > - char intf_name[RTE_KNI_NAMESIZE]; > - const struct rte_memzone *mz; > - struct rte_kni_memzone_slot *slot = NULL; > + struct rte_kni *kni; > + struct rte_tailq_entry *te = NULL; > + struct rte_kni_list *kni_list; > + > + kni_list = RTE_TAILQ_CAST(rte_kni_tailq.head, rte_kni_list); Can you move this below input validation, no need this assignment if API will fail because of wrong input. > if (!pktmbuf_pool || !conf || !conf->name[0]) > return NULL; > > /* Check if KNI subsystem has been initialized */ > - if (kni_memzone_pool.initialized != 1) { > + if (kni_fd < 0) { > RTE_LOG(ERR, KNI, "KNI subsystem has not been initialized. Invoke rte_kni_init() first\n"); > return NULL; > } > > - /* Get an available slot from the pool */ > - slot = kni_memzone_pool_alloc(); > - if (!slot) { > - RTE_LOG(ERR, KNI, "Cannot allocate more KNI interfaces; increase the number of max_kni_ifaces(current %d) or release unused ones.\n", > - kni_memzone_pool.max_ifaces); > - return NULL; > + rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK); > + > + TAILQ_FOREACH(te, kni_list, next) { > + kni = (struct rte_kni *) te->data; > + if (strncmp(conf->name, kni->name, RTE_KNI_NAMESIZE) == 0) > + break; > } This is rte_kni_get(), why not reuse it. You can create an version of it without lock. like _rte_kni_get() which you can call here. And rte_kni_get() rte_rwlock_read_lock(RTE_EAL_TAILQ_RWLOCK); _rte_kni_get() rte_rwlock_read_unlock(RTE_EAL_TAILQ_RWLOCK); <...> > + > + if (te == NULL) { > + goto unlock; > + } No need {} for single line. One more below.