From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh Jain Subject: Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver Date: Mon, 27 Mar 2017 10:24:51 +0530 Message-ID: <452b48e9-c94e-954e-6692-94692f456dfb@nxp.com> References: <1490004190-16892-1-git-send-email-shreyansh.jain@nxp.com> <20170324172227.5bdfac8c@platinum> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: , , To: Olivier Matz Return-path: Received: from NAM03-CO1-obe.outbound.protection.outlook.com (mail-co1nam03on0082.outbound.protection.outlook.com [104.47.40.82]) by dpdk.org (Postfix) with ESMTP id 16512F95E for ; Mon, 27 Mar 2017 06:48:58 +0200 (CEST) In-Reply-To: <20170324172227.5bdfac8c@platinum> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hello Olivier, On Friday 24 March 2017 09:52 PM, Olivier Matz wrote: > Hi Shreyansh, > > On Mon, 20 Mar 2017 15:33:09 +0530, Shreyansh Jain wrote: >> CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base. >> Stack mempool handler moved from lib/librte_mempool into drivers/mempool. >> >> With this patch, the Stack mempool handler registration is optional and >> toggled via the configuration file. In case disabled (N), it would imply >> request for creating of mempool would fail. >> >> Signed-off-by: Shreyansh Jain >> --- >> config/common_base | 5 + >> drivers/Makefile | 1 + >> drivers/mempool/Makefile | 36 +++++ >> drivers/mempool/stack/Makefile | 55 ++++++++ >> drivers/mempool/stack/rte_mempool_stack.c | 147 +++++++++++++++++++++ >> .../mempool/stack/rte_mempool_stack_version.map | 4 + >> lib/librte_mempool/Makefile | 1 - >> lib/librte_mempool/rte_mempool_stack.c | 147 --------------------- >> 8 files changed, 248 insertions(+), 148 deletions(-) >> create mode 100644 drivers/mempool/Makefile >> create mode 100644 drivers/mempool/stack/Makefile >> create mode 100644 drivers/mempool/stack/rte_mempool_stack.c >> create mode 100644 drivers/mempool/stack/rte_mempool_stack_version.map >> delete mode 100644 lib/librte_mempool/rte_mempool_stack.c > > > > I tried to pass the mempool autotest, and it issues a segfault. > I think the libraries are missing in rte.app.mk, so no handler is > registered. > > Adding the following code in lib/librte_mempool/rte_mempool_ops.c > fixes the crash. > > ops = rte_mempool_get_ops(mp->ops_index); > + if (ops == NULL || ops->alloc == NULL) > + return -ENOTSUP; > return ops->alloc(mp); > > Now that drivers are not linked to the mempool library, it can > happen that there is no handler. Could you please add this patch in your > patchset? Indeed. I will add this code set as first patch. Thanks for suggestion/fix. > > I also checked that compilation works in shared lib mode. It looks good, > but I think there will be a behavior change if CONFIG_RTE_EAL_PMD_PATH > is empty (default). This option is probably used by distros to indicate > where dpdk plugins are installed, and when it is set, all drivers of > this directory are loaded, so in that case it won't change. > > But when unset, the drivers have to be loaded manually, and with this > change, the mempool driver will have to be loaded with the -d eal option. > Could you please check what occurs in that case? At least see if it > displays a nice error or if it crashes. I suspect it will crash Ok. I will try this and if there is any issue, fix it. > > Also, the MAINTAINERS file should be updated. Yes, that is something I thought of updating but left it out before sending the patch. One confirmation: I assume that maintainers need to be added with "drivers/mempool/ring" and "drivers/mempool/stack" folders. Who should be marked as maintainer - You (because of existing lib/librte_mempool ownership) or I (because I have moved the code from lib/librte_mempool)? I think it would continue to be you but wanted to take your confirmation before adding the lines. > > > Thanks, > Olivier > > - Shreyansh