From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from na01-bl2-obe.outbound.protection.outlook.com (mail-bl2on0075.outbound.protection.outlook.com [65.55.169.75]) by dpdk.org (Postfix) with ESMTP id 7FE9E95DB for ; Thu, 4 Feb 2016 14:24:02 +0100 (CET) Authentication-Results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=caviumnetworks.com; Received: from localhost.localdomain (111.93.218.67) by BLUPR0701MB1714.namprd07.prod.outlook.com (10.163.85.140) with Microsoft SMTP Server (TLS) id 15.1.396.15; Thu, 4 Feb 2016 13:23:59 +0000 Date: Thu, 4 Feb 2016 18:53:41 +0530 From: Jerin Jacob To: "Hunt, David" Message-ID: <20160204132340.GA13777@localhost.localdomain> References: <1453829155-1366-1-git-send-email-david.hunt@intel.com> <1453829155-1366-2-git-send-email-david.hunt@intel.com> <20160128175216.GB11992@localhost.localdomain> <56B20BA6.3090204@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <56B20BA6.3090204@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Originating-IP: [111.93.218.67] X-ClientProxiedBy: MAXPR01CA0038.INDPRD01.PROD.OUTLOOK.COM (25.164.146.138) To BLUPR0701MB1714.namprd07.prod.outlook.com (25.163.85.140) X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1714; 2:5iWYlH0XEG6EbSPQz0pcsPLWMHVWlg++LkWVmeh75CSUDMACkxZTg57en8mSyyUmE+kFsA70ynDEVb9xb1FYd/D9UEvbEz4qx9yx4a9SretdgKx79MXRXw0iBViVLercEejg26j/KTXXgshZUtndqQ==; 3:Ww2F4hJdIbsUsc7NU0ttQThRuHvMyzppQghVKcUvxpeQTCDrz9jw82u+VsxAFavLO3xL7//4R3h8ZaloG41abzfPkEpCqyS9qmcQMDqBuifd7WW24xLG8FNsg6p8OQzH; 25:CdmFGX4A/sv+IWwbJp2v/zHGIxjSwzAG0EG0/9AZP0Ou5oqv+f0Zu0rPxsRMW+sDuVmB/FG/rVuQHZzZycq5CfXBf94B6H8hc1RxCFmBuwXbCAJ08s/LP11wIDHOtU99cBNnTl8dCUbIjuO8x4EdjHq9/DVsUoIAe2+2CS/Qss4EJ2FYMwZYPrdi4tHstHV/h4+w7QCes2VhiPMKJdQ+pVsUIuzoOK4gSyeMPAdd76SjJ7buOkNsW/0jc+cE0/p0ViDupPslaXpXFBi23HBtU+uBDiLQqGKvoCD2C3gN8cU= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BLUPR0701MB1714; X-MS-Office365-Filtering-Correlation-Id: 183156b8-7b64-4dab-6dd2-08d32d6671e2 X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1714; 20:uqru9nmXnc+jFUAZY+Vw2ANmA9mmm7g52eFwZ2imf1NwFJxmO+XO0H0AdIcrjZGT639r+m/Dfs8kRJyu1EY/cnnbTZWJvmu3c+yGz953xJv3PSXJoRMva5tblUAxFGADhwSbguwi4pF4LZ7s9SPOw+NOi59e5Hx2c52gbbQYJcAxFW8bR93pcVUQpQl9lp0d6iWHMc2faDl+wy47n3lHmKMWSts2ZpzodZ221XBbhENOY/B/Y3kXPRrDxULjErgePG8jN69K5bgVEhG9Y5t38wSNSixQ4jy1UP80+9qnEtOS4WsKQZp/JE+Hd2MT77QFiOuw18IUAgaUVQss/q6KOk2jar0reCCEg1tb3jqRIJeW4A6JwcAzhNTe4MLiO5QwbBmDU9Fnnx/6guyHJ7RC5tIM9ivcl6+jl9zPEv0bOSOwa3YWSUKUABXNcuxiygAduGMhexzcfNHqWoDiv0j/Phy0vYSGExkn7uIwvoovXpLg1IxjeBlC1OEjVFDxzUcXToXPtyOSQdVcnMbrXsX7gerT3ozdSQ6LakUoAgD+JeQUbk8xuU6YI9z61tgOjVPbhpf9fQJAx17eJmWTSrfChS0fR569hT3oS1RxrWr/GxU= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001); SRVR:BLUPR0701MB1714; BCL:0; PCL:0; RULEID:; SRVR:BLUPR0701MB1714; X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1714; 4:9ECPcPdLnkqihG/4pbPOL9zqNWcX73vGj2WvSo/DyqhdmCy2ZVAGRuJ0hY0ATHzofe4bs7S9UX6ChB+kcAakaIRbf8YmS2n8Fyy8VIyFjj98fn7E88LDclfV7mf+gUl93hcdFPxqoU/QuOWK21ZDGsEJzMWLIUXNKvb1HWIG8rOQFaubgi28TnZJUk/iXmX0RYzp9bT+MbykMj4p4FBjIVVGSl6UrebMIeyqBisj1GJz9hDDq/IpQke+73jQJkQq4k+Kbs1hdKMPL/aj3+oZHn/2VqxRreITm00DR2Y5cMyCzj+Ue27EIiNAwi7SuLaUk97MBW/eVmoaeOGSu++1xVd+xDhHmAqdVTGIPqOc9GLCtzjoQJ2v23UEJbW+i61g X-Forefront-PRVS: 084285FC5C X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(6009001)(6069001)(24454002)(164054003)(479174004)(4326007)(77096005)(46406003)(93886004)(5008740100001)(87976001)(47776003)(19580395003)(83506001)(19580405001)(110136002)(586003)(5001960100002)(6116002)(2950100001)(3846002)(50466002)(61506002)(189998001)(76176999)(50986999)(54356999)(42186005)(1076002)(4001350100001)(1096002)(5009440100003)(86362001)(122386002)(97756001)(33656002)(40100003)(66066001)(92566002)(2906002)(23726003)(7099028); DIR:OUT; SFP:1101; SCL:1; SRVR:BLUPR0701MB1714; H:localhost.localdomain; FPR:; SPF:None; MLV:sfv; LANG:en; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; BLUPR0701MB1714; 23:2VrpfSSi1M+9JwP8lYoMMidQjkTc7v9Dmt/0CkW?= =?us-ascii?Q?00TLQMcWRe8ihpUD1HaSaStJacUkuMyC8BrkXmp7AeMP/hJBEvUlfq2ZpCkT?= =?us-ascii?Q?MeIPB6ttWCpLK//AAKJZaMOSfegizbj6Mv1kfvTOt1omNyi4AiJpARCl8iBm?= =?us-ascii?Q?Ui9Y9UYe6vqY2VhwErRszSEvnA54DWK3ujTfRh5ljtVbzyCutRxG7oaFzdh7?= =?us-ascii?Q?TzZSe7yWZN3WAFtCDFikvSmeMD8t1gztbNOHUhpstJ0TB0ME1ZzQxHct+IOx?= =?us-ascii?Q?rIKf9ytcIkiWB/EzmPRpuRI4apRu7IUbdkSHPAQL7TLNwFs/FkLK8wCKVFBH?= =?us-ascii?Q?k/Q1ZIT2L1/atxUMXm4WvoxhM8P0f+IER62KCONfj/ayECboQCkH+MT+12Bk?= =?us-ascii?Q?tacha0xMewF9DcW27Ekvg2kXTsPVsY3L3Ffhu6acyarbwIdfhyX8kckc3Fvd?= =?us-ascii?Q?QLVNRZhLrAgF9n70es+BeTcEsMtMBmc2Uyz7rOPADp5qd3rq/ulE9oGeCMcK?= =?us-ascii?Q?I0x0B9WygNojPTJzQSTcYsjTF5Z8/PEYmDhtJMrAiz3ur0d8CDLCCLWGMZrt?= =?us-ascii?Q?uUcgJZbpYBexMwLjcffmHcvpKeQsmOPy32lEL5IOiNCjbC74iGhjzdbnboTG?= =?us-ascii?Q?HqAB4tTazRvJiBbx7GvKunuA+QgPCFclPibLwjYTnGAeLhv622ve9QID7kV/?= =?us-ascii?Q?yajzguZ0nlmrxwvjLsMp3RIVN3w6eek6QycKadeGYwCglGmDAdUqXZgqDRub?= =?us-ascii?Q?59l1N62hFKUIrg03/I6m/PA1mW1SfpdWw3rjqqwfwpfhM7Q4ofA+PdF+F2kM?= =?us-ascii?Q?sgCzrx+Vz7fTKrejFMWXHhCNgttO9nJ5M8TzV6QsbVy2ipfjQ0pUUG1J5izQ?= =?us-ascii?Q?ci+SvGuEcuQut0tum16cVGrcwaEvd38E8p7bWA3Aj/R1FT7D7vk8DyLxReka?= =?us-ascii?Q?HSy4o1+8OM8Da5J/Lpva39PS0eTXWhRVT99mhyg0MZp08KBpCRh474IGUrkp?= =?us-ascii?Q?C7ANLw2k4VpOYtTC96lo/1Jfwgn0LYZ47esBf0zoXt4Xi8tmLqoeglPj5Zb6?= =?us-ascii?Q?yXKNyAkAwRE0TnvTUF9yG012AsOfdTqPLnIekriyClLNJZ5BiSw=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1; BLUPR0701MB1714; 5:4CW/vgNuWeoBJ95wKWVdXvjD2gL1dcevxQiue6mFkBbY8tW6DB8WWUHXlIiH/QwGz4Mja7WOyyDwnHbsOa883RJYShJFOEJobrElk8CR/mlYPCYny/9FMyItqzG5JcqIQUztxgzc2ygUSAY/hcUW1g==; 24:sIj79CuC38tekiMYxvzEqw5Ajeea50m2krh2XX1jXPypP+7cssqq8GLJu91Zwu+qcKtYnr4YBHMTlzEaTCxIJDJqBBtjOPcn5iJQC3WAzH4= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Feb 2016 13:23:59.8856 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR0701MB1714 Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 1/5] mempool: add external mempool manager support 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, 04 Feb 2016 13:24:03 -0000 On Wed, Feb 03, 2016 at 02:16:06PM +0000, Hunt, David wrote: > On 28/01/2016 17:52, Jerin Jacob wrote: > >On Tue, Jan 26, 2016 at 05:25:51PM +0000, David Hunt wrote: > >>Adds the new rte_mempool_create_ext api and callback mechanism for > >>external mempool handlers > >> > >>Modifies the existing rte_mempool_create to set up the handler_idx to > >>the relevant mempool handler based on the handler name: > >> ring_sp_sc > >> ring_mp_mc > >> ring_sp_mc > >> ring_mp_sc > >> > >>Signed-off-by: David Hunt > >>--- [snip] > >>+ if (flags & MEMPOOL_F_USE_STACK) > >>+ mp->handler_idx = rte_get_mempool_handler("stack"); > >>+ else if (flags & (MEMPOOL_F_SP_PUT | MEMPOOL_F_SC_GET)) > >>+ mp->handler_idx = rte_get_mempool_handler("ring_sp_sc"); > >>+ else if (flags & MEMPOOL_F_SP_PUT) > >>+ mp->handler_idx = rte_get_mempool_handler("ring_sp_mc"); > >>+ else if (flags & MEMPOOL_F_SC_GET) > >>+ mp->handler_idx = rte_get_mempool_handler("ring_mp_sc"); > >>+ else > >>+ mp->handler_idx = rte_get_mempool_handler("ring_mp_mc"); > >>+ > > > >Why still use flag based selection? Why not "name" based? See below > >for more description > > > The old API does not have a 'name' parameter, so needs to work out which > handler to use based on the flags. This is not necessary in the new API > call, and so it uses the "name" based index. > I agree. But the old API to new API mapping still can be done like below, rte_mempool_create -> rte_mempool_create_ext(..,"ring_mp_mc") [snip] > > >>+ /* init the mempool structure */ > >>+ mp = mz->addr; > >>+ memset(mp, 0, sizeof(*mp)); > >>+ snprintf(mp->name, sizeof(mp->name), "%s", name); > >>+ mp->phys_addr = mz->phys_addr; > >>+ mp->size = n; > >>+ mp->flags = flags; > >>+ mp->cache_size = cache_size; > >>+ mp->cache_flushthresh = CALC_CACHE_FLUSHTHRESH(cache_size); > >>+ mp->private_data_size = private_data_size; > >>+ mp->handler_idx = handler_idx; > >>+ mp->elt_size = elt_size; > >>+ mp->rt_pool = rte_mempool_ext_alloc(mp, name, n, socket_id, flags); > > > > > >IMO, We can avoid the duplicaition of above code with rte_mempool_create. > >i.e rte_mempool_create -> rte_mempool_create_ext(..,"ring_mp_mc") > > > rte_mempool_create is not really a subset of rte_mempool_create_ext, so > doing this would not be possible. I did have a look at this before pusing > the patch, but the code was so different in each case I decided to leave > them as is. Maybe break out the section that sets up the mempool structure > in to a separate functinon? Yes, Their are a lot of common code between rte_mempool_create/rte_mempool_xmem_create and rte_mempool_create_ext. IMO, we need to converge these functions. Otherwise, a new feature in mempool would call for changing in both places and it's difficult to maintain. In my view, both external and internal pool manager should have the same code for creation and just the backend put/get/alloc function pointers will be different to maintain the functional consistency. Thanks, comments appreciated. Jerin