From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-f194.google.com (mail-pg1-f194.google.com [209.85.215.194]) by dpdk.org (Postfix) with ESMTP id 1D5055323 for ; Wed, 26 Sep 2018 15:31:56 +0200 (CEST) Received: by mail-pg1-f194.google.com with SMTP id v133-v6so6410822pgb.2 for ; Wed, 26 Sep 2018 06:31:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nfware-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=2HSwQ7tRZsKAYaamKTaKz0eV+VpSH8P75R/uvB8f8tk=; b=dEqnX7n8kQp9wXzH6dH+mqS2C18mUe8mOEF19p200JNxpEQa8alopth66MmKUkeneA /8lxZC30dsWWGCb/yo63L+nuoE7lMftUImfuyvH7xySPygFOfcrZjq4Qz5KX8SSf57gy pQ0Azffu90iVrcxqnOAk8unoDOd+XGd2Jdb76pwY5r4SwmZDGzKyVAsoLaPaTS/k+xD2 VgipltbRdMFtjkAnuCVQZJIFIJlyNtQR/adMmabOpRqabdgI03J2GYH/1rgZdph3crKg F0i4mo1Y7Sx/AyuRZE9QD3ZmhqEpBAtvCPOgatwssIHAzAfGIdgvyqYF3o7Mjv0hfPu/ B01A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=2HSwQ7tRZsKAYaamKTaKz0eV+VpSH8P75R/uvB8f8tk=; b=AXj1Y3eUpUdtnnu1Fvu0XTa2OVwOxISRzekB+MU9Di+qfOfp3ywTWIIjMlJ3WdNCti XJtiWmd6XSxyngOU74qSUf2taCAElOYMXB88NjBry5mvGNWd2cizxt3B4MxAsccrRX7X qhDVwfnTFBPnb7BlI/CIR431fSeniPnbMC5z6yU6Eu2SdenC8m+8bzUZig0dWtQaSB7j x9uUvuJcp3mfaqYpwKmmCEGiL1dOMOPHSB1rzXcyjx6CJ6OOwr8alT93pZ2/qoTKJcAP L7RkUEz3JMSTC7hcCxNnbiCaoHdkAEjA9g70/fcmFgq7BseIku4H/tnbp6Ar6jV+yYEk HXhA== X-Gm-Message-State: ABuFfojpMiALvcUVBLHZqA6XjZsc8AMlvCujRqatQzjGwZTHT6uQErmz q76Hh1BFIu/QxC47ASiLw87xsFTHoRxCL4ONzXeiMavrQCQ= X-Google-Smtp-Source: ACcGV62o4sCCkRKWOJQPf+TPO+5EcLttBmH5+0Y64oowiIGbKFKFbKqg1zYKQHYA5I2B5sfh75JiYQ8K1VI8b+duhqI= X-Received: by 2002:a62:a116:: with SMTP id b22-v6mr5189251pff.99.1537968715202; Wed, 26 Sep 2018 06:31:55 -0700 (PDT) MIME-Version: 1.0 References: <20180802142522.57900-1-iryzhov@nfware.com> <20180923191202.64896-1-iryzhov@nfware.com> <594e0f5a-7f4f-5713-3cdf-d41e492e0a04@intel.com> In-Reply-To: <594e0f5a-7f4f-5713-3cdf-d41e492e0a04@intel.com> From: Igor Ryzhov Date: Wed, 26 Sep 2018 16:31:44 +0300 Message-ID: To: Ferruh Yigit Cc: dev@dpdk.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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 13:31:56 -0000 Hi Ferruh, Will fix it today. Igor On Wed, Sep 26, 2018 at 1:41 PM Ferruh Yigit wrote: > 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. >