From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 62F05A00C3; Tue, 20 Sep 2022 13:30:16 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4A8C540DFB; Tue, 20 Sep 2022 13:30:16 +0200 (CEST) Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) by mails.dpdk.org (Postfix) with ESMTP id 805524069B for ; Tue, 20 Sep 2022 13:30:15 +0200 (CEST) Received: by mail-lf1-f51.google.com with SMTP id a3so3220078lfk.9 for ; Tue, 20 Sep 2022 04:30:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date; bh=UcVNCprnC/QE8AeT1vDnoKkrV/fxEbO2yuydmK7oti0=; b=FCpIIQSdTEbwURgbyEAoexV2Rad3d6tC2ABRaVeWiIuOD5UhG/WvNFJkHwlc1fwo6F yJZSkInJ1D+p/DsS4YMJUNRvPw1mOewvUHo0mMGC9L8/qEfdcB2sJEEogF2qQevfB00i By/5yH0bexByprhyP0F1BXhwR3i5Kn5AahBfOBhcdnZefAj3lpc69CfcptGS3kmU+24k I8jIwlRvVoTtQgDN9Fr0M/COZO4OyRrPh7SJFI8TwVLf+0KtyXSt9Nv+4MXbDcvJAXAw MEromQmYmc3HAU4ohRdZU5XibTaMadCCOzKgQ0GDfyVRvfYjpnXURCcE6rV9qVMa9ni7 KWwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date; bh=UcVNCprnC/QE8AeT1vDnoKkrV/fxEbO2yuydmK7oti0=; b=AIk/s/p72Bdwz6R9nwePgOnCftidJNdBhXQnpugHvXU/r3lbAhy8I7FLWnGOKC/h7M WnxuuSyw+fmZKSOPjZnL3QBYXUjuogM2KtodSpAzohvsQPjDjtADOzfSGJlIVlQJceC7 4gdFwqFYSbw5DwpcVN6AwwIkM6MtUi8ORVTO+PG33O9InOcx9U7kArxsbDeUP+vOIa5i XOD3FyDAksZH8e0la+4BLJP578xalyEcKIj7HLBAvq4WURpTDdfX/qM8cjtdIqVXCScW mFjxETe7GHvqwZCSdm/DjB8JsYzWJXYhPKO/PoVKW4cFIK3FhdREaBkdDidKS22zI386 7w/w== X-Gm-Message-State: ACrzQf2fyPxWKvvTX4y+fcx45NWpL4dEVNCTlJpxcmgciNeIyt21dqnT Py/Ep2nysRp6T/R/dUSvu+8= X-Google-Smtp-Source: AMsMyM6d67AzYIUTbW7cyIC9056K5qtqi4tRyRrqI5cDe8yOf0G/a3YLKcp/sHmc+n4n6t+kHzQDMw== X-Received: by 2002:a05:6512:3b09:b0:49f:3fca:2c87 with SMTP id f9-20020a0565123b0900b0049f3fca2c87mr7793474lfv.603.1663673414862; Tue, 20 Sep 2022 04:30:14 -0700 (PDT) Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id u8-20020a05651220c800b0049fe5bb052esm176363lfr.242.2022.09.20.04.30.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 20 Sep 2022 04:30:14 -0700 (PDT) Date: Tue, 20 Sep 2022 14:30:13 +0300 From: Dmitry Kozlyuk To: Chengwen Feng Cc: , , , Subject: Re: [PATCH 1/8] memarea: introduce memory area library Message-ID: <20220920143013.7624f36d@sovereign> In-Reply-To: <20220920034643.55476-2-fengchengwen@huawei.com> References: <20220721044648.6817-1-fengchengwen@huawei.com> <20220920034643.55476-1-fengchengwen@huawei.com> <20220920034643.55476-2-fengchengwen@huawei.com> X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 2022-09-20 03:46 (UTC+0000), Chengwen Feng: > The memarea library is an allocator of variable-size object. It is a > collection of allocated objects that can be efficiently alloc or free > all at once, the main features are as follows: > a) it facilitate alloc and free of memory with low overhead. Yet, the overhead is 64B per element, just like rte_malloc. > b) it provides refcnt feature which could be useful in some scenes. Are you sure refcnt should be in this library? I've expressed my concerns here: https://inbox.dpdk.org/dev/CAEYuUWBpC-9dCqKJ0LZi6RkCUwyeYEghLRBMBUBtUx4Ljg+pAQ@mail.gmail.com There are more unanswered questions in that mail, it would be good to clarify them before reviewing these patches in order to understand all the intentions. > +static int > +memarea_check_param(const struct rte_memarea_param *init) > +{ > + size_t len; > + > + len = strnlen(init->name, RTE_MEMAREA_NAMESIZE); > + if (len == 0 || len >= RTE_MEMAREA_NAMESIZE) { > + RTE_LOG(ERR, MEMAREA, "memarea name invalid!\n"); > + return -EINVAL; > + } Please check init->name == NULL first. > +struct rte_memarea * > +rte_memarea_create(const struct rte_memarea_param *init) > +{ [...] > + RTE_LOG(ERR, MEMAREA, "malloc memarea management obj fail!\n"); In all error messages, it would be useful to provide details: the name of the area, what size was requested, etc. > +/** > + * Memarea memory source. > + */ > +enum rte_memarea_source { > + /** Memory source comes from system API (e.g. malloc). */ > + RTE_MEMAREA_SOURCE_SYSTEM_API, > + /** Memory source comes from user-provided address. */ > + RTE_MEMAREA_SOURCE_USER_ADDR, > + /** Memory source comes from user-provided memarea. */ > + RTE_MEMAREA_SOURCE_USER_MEMAREA, > + > + RTE_MEMAREA_SOURCE_BUTT DPDK enumerations must not include an item to hold the element count, because it is harmful for ABI (e.g. developers create arrays of this size and when a new item is added in a new DPDK version, the array overflows). If it's supposed to mean "the end of item list", the proper word would be "last" or "max" BTW :) > +}; > + > +struct rte_memarea { > + void *private_data; /**< private management data pointer. */ > +}; Jerin and Stephen suggested to make the structure opaque, i.e. only declare the struct and define it privately. It would reduce ABI and simplify allocation. Any justification to expose it?