From mboxrd@z Thu Jan 1 00:00:00 1970 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) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 25464400056; Mon, 12 Mar 2018 06:53:35 +0000 (UTC) Received: from [192.168.38.17] (84.52.114.114) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Mon, 12 Mar 2018 06:53:30 +0000 To: santosh , CC: Olivier MATZ 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> From: Andrew Rybchenko Message-ID: <5017d372-6af7-c500-26d4-27acc38d76a0@solarflare.com> Date: Mon, 12 Mar 2018 09:53:26 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <9616c281-5a6c-c9fa-8346-bccdce9df7cd@caviumnetworks.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB X-Originating-IP: [84.52.114.114] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23714.003 X-TM-AS-Result: No--10.443300-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1520837616-Y2ecVVjdDH1H Subject: Re: [dpdk-dev] [PATCH v1 1/9] mempool: add op to calculate memory size to be allocated 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: Mon, 12 Mar 2018 06:53:37 -0000 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.