From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Rybchenko Subject: Re: [PATCH v1 1/9] mempool: add op to calculate memory size to be allocated Date: Mon, 12 Mar 2018 09:53:26 +0300 Message-ID: <5017d372-6af7-c500-26d4-27acc38d76a0@solarflare.com> References: <1516713372-10572-1-git-send-email-arybchenko@solarflare.com> <1520696382-16400-1-git-send-email-arybchenko@solarflare.com> <1520696382-16400-2-git-send-email-arybchenko@solarflare.com> <9616c281-5a6c-c9fa-8346-bccdce9df7cd@caviumnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: Olivier MATZ To: santosh , Return-path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id CCEA82B9C for ; Mon, 12 Mar 2018 07:53:36 +0100 (CET) In-Reply-To: <9616c281-5a6c-c9fa-8346-bccdce9df7cd@caviumnetworks.com> Content-Language: en-GB List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 03/11/2018 03:51 PM, santosh wrote: > Hi Andrew, > > > On Saturday 10 March 2018 09:09 PM, Andrew Rybchenko wrote: >> Size of memory chunk required to populate mempool objects depends >> on how objects are stored in the memory. Different mempool drivers >> may have different requirements and a new operation allows to >> calculate memory size in accordance with driver requirements and >> advertise requirements on minimum memory chunk size and alignment >> in a generic way. >> >> Bump ABI version since the patch breaks it. >> >> Signed-off-by: Andrew Rybchenko >> --- >> RFCv2 -> v1: >> - move default calc_mem_size callback to rte_mempool_ops_default.c >> - add ABI changes to release notes >> - name default callback consistently: rte_mempool_op__default() >> - bump ABI version since it is the first patch which breaks ABI >> - describe default callback behaviour in details >> - avoid introduction of internal function to cope with depration >> (keep it to deprecation patch) >> - move cache-line or page boundary chunk alignment to default callback >> - highlight that min_chunk_size and align parameters are output only >> > [...] > >> diff --git a/lib/librte_mempool/rte_mempool_ops_default.c b/lib/librte_mempool/rte_mempool_ops_default.c >> new file mode 100644 >> index 0000000..57fe79b >> --- /dev/null >> +++ b/lib/librte_mempool/rte_mempool_ops_default.c >> @@ -0,0 +1,38 @@ >> +/* SPDX-License-Identifier: BSD-3-Clause >> + * Copyright(c) 2016 Intel Corporation. >> + * Copyright(c) 2016 6WIND S.A. >> + * Copyright(c) 2018 Solarflare Communications Inc. >> + */ >> + >> +#include >> + >> +ssize_t >> +rte_mempool_op_calc_mem_size_default(const struct rte_mempool *mp, >> + uint32_t obj_num, uint32_t pg_shift, >> + size_t *min_chunk_size, size_t *align) >> +{ >> + unsigned int mp_flags; >> + int ret; >> + size_t total_elt_sz; >> + size_t mem_size; >> + >> + /* Get mempool capabilities */ >> + mp_flags = 0; >> + ret = rte_mempool_ops_get_capabilities(mp, &mp_flags); >> + if ((ret < 0) && (ret != -ENOTSUP)) >> + return ret; >> + >> + total_elt_sz = mp->header_size + mp->elt_size + mp->trailer_size; >> + >> + mem_size = rte_mempool_xmem_size(obj_num, total_elt_sz, pg_shift, >> + mp->flags | mp_flags); >> + > Looks ok to me except a nit: > (mp->flags | mp_flags) style expression is to differentiate that > mp_flags holds driver specific flag like BLK_ALIGN and mp->flags > has appl specific flags.. is it so? If not then why not simply > do like: > mp->flags |= mp_flags. In fact it does not mater a lot since the code is removed in the patch 3. Here it is required just for consistency. Also, mp argument is a const which will not allow to change its members.