From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 17901A058A; Fri, 17 Apr 2020 15:04:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7F3CC1E555; Fri, 17 Apr 2020 15:04:55 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 8C4491E54D for ; Fri, 17 Apr 2020 15:04:53 +0200 (CEST) IronPort-SDR: 1wPOeduo3EvHLZcTyus/7Mxebzl/AQbPT0frA1FdKuzg9a6EHdDAYljcG5qq2SVAtzRIsyfton 0gZXx9kBwi1Q== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Apr 2020 06:04:52 -0700 IronPort-SDR: Dyu0Y8j1aoi++YCQKrZspwCdEmKJTqiTgysFJomLzWPpggJXJ/vP9hO9OOSgNuGIxYFgAuY97f 9lFJjvQrUvxA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,395,1580803200"; d="scan'208";a="278373922" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.252.62.102]) ([10.252.62.102]) by fmsmga004.fm.intel.com with ESMTP; 17 Apr 2020 06:04:50 -0700 To: Dmitry Kozlyuk , dev@dpdk.org Cc: "Dmitry Malloy (MESHCHANINOV)" , Narcisa Ana Maria Vasile , Fady Bader , Tal Shnaiderman , Bruce Richardson References: <20200410164342.1194634-1-dmitry.kozliuk@gmail.com> <20200414194426.1640704-1-dmitry.kozliuk@gmail.com> <20200414194426.1640704-8-dmitry.kozliuk@gmail.com> From: "Burakov, Anatoly" Message-ID: <4aa4feaa-9d6b-7a6a-5bc1-75d164548b47@intel.com> Date: Fri, 17 Apr 2020 14:04:48 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <20200414194426.1640704-8-dmitry.kozliuk@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 07/10] eal: extract common code for memseg list initialization 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 14-Apr-20 8:44 PM, Dmitry Kozlyuk wrote: > All supported OS create memory segment lists (MSL) and reserve VA space > for them in a nearly identical way. Move common code into EAL private > functions to reduce duplication. > > Signed-off-by: Dmitry Kozlyuk > --- > lib/librte_eal/common/eal_common_memory.c | 54 ++++++++++++++++++ > lib/librte_eal/common/eal_private.h | 34 ++++++++++++ > lib/librte_eal/freebsd/eal_memory.c | 54 +++--------------- > lib/librte_eal/linux/eal_memory.c | 68 +++++------------------ > 4 files changed, 110 insertions(+), 100 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c > index cc7d54e0c..d9764681a 100644 > --- a/lib/librte_eal/common/eal_common_memory.c > +++ b/lib/librte_eal/common/eal_common_memory.c > @@ -25,6 +25,7 @@ > #include "eal_private.h" > #include "eal_internal_cfg.h" > #include "eal_memcfg.h" > +#include "eal_options.h" > #include "malloc_heap.h" > > /* > @@ -182,6 +183,59 @@ eal_get_virtual_area(void *requested_addr, size_t *size, > return aligned_addr; > } > > +int > +eal_reserve_memseg_list(struct rte_memseg_list *msl, > + enum eal_mem_reserve_flags flags) This and other similar places in this and other patches: i don't think using enums for this purpose (i.e. to hold multiple values at once) is good practice. I would suggest replacing with int. Also, i don't think "eal_reserve_memseg_list" is a particularly good or descriptive name (and neither is "eal_alloc_memseg_list" for that matter). Suggestion: "eal_memseg_list_create" (or "_init") and "eal_memseg_list_alloc"? > +{ > + uint64_t page_sz; > + size_t mem_sz; > + void *addr; > + > + page_sz = msl->page_sz; > + mem_sz = page_sz * msl->memseg_arr.len; > + > + addr = eal_get_virtual_area(msl->base_va, &mem_sz, page_sz, 0, flags); > + if (addr == NULL) { > + if (rte_errno == EADDRNOTAVAIL) > + RTE_LOG(ERR, EAL, "Cannot reserve %llu bytes at [%p] - " > + "please use '--" OPT_BASE_VIRTADDR "' option\n", > + (unsigned long long)mem_sz, msl->base_va); > + else > + RTE_LOG(ERR, EAL, "Cannot reserve memory\n"); > + return -1; > + } > + msl->base_va = addr; > + msl->len = mem_sz; > + > + return 0; > +} > + > +int > +eal_alloc_memseg_list(struct rte_memseg_list *msl, uint64_t page_sz, > + int n_segs, int socket_id, int type_msl_idx, bool heap) > +{ > + char name[RTE_FBARRAY_NAME_LEN]; > + > + snprintf(name, sizeof(name), MEMSEG_LIST_FMT, page_sz >> 10, socket_id, > + type_msl_idx); > + if (rte_fbarray_init(&msl->memseg_arr, name, n_segs, > + sizeof(struct rte_memseg))) { > + RTE_LOG(ERR, EAL, "Cannot allocate memseg list: %s\n", > + rte_strerror(rte_errno)); > + return -1; > + } > + > + msl->page_sz = page_sz; > + msl->socket_id = socket_id; > + msl->base_va = NULL; It probably needs to be documented that eal_alloc_memseg_list must be called before eal_reserve_memseg_list. > + msl->heap = heap; > + > + RTE_LOG(DEBUG, EAL, "Memseg list allocated: 0x%zxkB at socket %i\n", > + (size_t)page_sz >> 10, socket_id); > + > + return 0; > +} > + > static struct rte_memseg * > virt2memseg(const void *addr, const struct rte_memseg_list *msl) > { -- Thanks, Anatoly