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 66BF2A034C; Tue, 30 Aug 2022 14:41:45 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0BCA640F18; Tue, 30 Aug 2022 14:41:45 +0200 (CEST) Received: from mail-ot1-f45.google.com (mail-ot1-f45.google.com [209.85.210.45]) by mails.dpdk.org (Postfix) with ESMTP id 9123340F17 for ; Tue, 30 Aug 2022 14:41:43 +0200 (CEST) Received: by mail-ot1-f45.google.com with SMTP id br15-20020a056830390f00b0061c9d73b8bdso7958648otb.6 for ; Tue, 30 Aug 2022 05:41:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc; bh=lEnXBWcDDsdioMS9M64c8n+NLwNaH21bskv9s3OLJXQ=; b=mwfmCOUCdsxSuSlv8GrjxRE4r4IciygmvBhZ5Jh0AHI6l0fd3Dc4+zIC11h4CNu1oK N9+lsIc89iubykFCib9osWldzajZLjOtr2E83YLxLXbSPDz7z5YO5HlNGPoYQEXLyqGS jw6RUOmRH+srhZbYQfFqm+bWWQ5fizI9i/uEh85K/8YI2ukAcCVG7wpt5WXnDgXANbXp Ds4VMsB9F5al9QGFiL3Bjk99/VUGr2rKMCz2ICNeU+utRT+vRNu14R6kvxQR2iIMVxkY sKD5Fzqtk93g/6ViP9ZHGyKllitMu+16CUHPgP2b0Zz6Y+bpbe8HUZKNkTDkAqgahbkX dIYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=lEnXBWcDDsdioMS9M64c8n+NLwNaH21bskv9s3OLJXQ=; b=kMW0FVHjDTwAwCs7XP7csREA2zpzk4buTM4L2u3ZY7Yt1rxY1RK36kLTw33vANWNFO feaWDXhQIfmqXEuXRJgh9bB7tNC5ysiF+lVy5sW5In54kVpkBDwc65BnaKOiLFiILPmy 2soXTSHi6TI9EId9yVuMVxO/+VH67CH0vyldqoRqneKy2AFZ8rGy584iXSgu1H6J4XYP Ll13QuI+hANjqre+eWlfKGimyBw0qf5WWeXIlUVKg8bfqmBl3HWhDdcZ3f1TFlqZ4tgU 3rKS4nWKrf3/a5Q7CQI585TPFF4UwSBEqE/jtJjo5gSrnywIC0O9gCc0ZHt/0vMUYFYz JJvQ== X-Gm-Message-State: ACgBeo0ZrrbZW3z3zZXYqlbSJN88pJeHUVAIjKxiMZZmB9XRkNz1tdbn s7mVSeccoslMJCTDb6p791j9YN9frbROTpjTgXUVtCDf X-Google-Smtp-Source: AA6agR5DTqiTsFj095qe7/tM8zLAmidMkepxVqyFzfUvaLWFOlhQI8fDRPwpXYyM4yIHiHOFfJCDEu8930qk3KZbjW8= X-Received: by 2002:a05:6830:418b:b0:637:3897:e279 with SMTP id r11-20020a056830418b00b006373897e279mr8243975otu.78.1661863302820; Tue, 30 Aug 2022 05:41:42 -0700 (PDT) MIME-Version: 1.0 References: <20220721044648.6817-1-fengchengwen@huawei.com> In-Reply-To: <20220721044648.6817-1-fengchengwen@huawei.com> From: Dmitry Kozlyuk Date: Tue, 30 Aug 2022 15:41:31 +0300 Message-ID: Subject: Re: [RFC] memarea: introduce memory area library To: Chengwen Feng Cc: Thomas Monjalon , dpdk-dev Content-Type: multipart/alternative; boundary="000000000000d47ab405e774b32b" 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 --000000000000d47ab405e774b32b Content-Type: text/plain; charset="UTF-8" > > Note: > a) the memarea is oriented towards the application layer, which could > provides 'region-based memory management' [1] function. > Judging from the API, this library would rather provide an interface to a generic allocator over a fixed memory extent, because it offers freeing of specific elements, and thus must track them. So it's more than RBMM. Is this intended? It's a very interesting RFC anyway, just trying to understand the scope. b) the eal library also provide memory zone/heap management, but these > are tied to huge pages management. > [...] > + * The memarea is a collection of allocated objects that can be > efficiently > + * alloc or free all at once, the main feature are as follows: > + * a) it facilitate alloc and free of memory with low overhead. > + * [...] > + * c) it supports MT-safe as long as it's specified at creation time. > These two bullets seem to add the most value compared to DPDK heap API. DPDK heap overhead is at least 64 bytes per allocation (sizeof malloc_elem), so I assume memarea aims at a large number of small elements. > +struct rte_memarea_param { > + char name[RTE_MEMAREA_NAMESIZE]; /**< Name of memarea */ > + enum rte_memarea_source source; /**< Memory source of memarea */ > + uint64_t size; /**< Size (byte) of memarea */ > Is it an upper limit or a guaranteed size? It probably depends on the source: guaranteed for USER_ADDR, upper limit for SYSAPI (or it would be no different from USER_ADDR), not sure about USER_MEMAREA. Do you envision memarea as always limited? Generic allocators usually have means of adding extents, even if this one doesn't currently. Nit: size is uint64_t here but uint32_t in rte_memarea_allloc(). Should be size_t in both places. > + uint32_t align; /**< Align of allocated object */ > + /** Indicates whether the memarea should be MT-safe */ > + bool mt_safe; > + /** Indicates whether the memarea is visible to multiple process. > + * If the memory source is RTE_MEMAREA_SOURCE_USER_ADDR, this filed > + * depends on user settings and must be set. > + * If the memory source is RTE_MEMAREA_SOURCE_SYSAPI or > + * RTE_MEMAREA_SOURCE_USER_MEMAREA, this filed does not need to be > set. > + */ > + bool mp_visible; > + /** User provided address, this field is valid only when source > + * is set to RTE_MEMAREA_SOURCE_USER_ADDR. > + */ > + void *user_addr; > + /** User provided memarea, this field is valid only when source > + * is set to RTE_MEMAREA_SOURCE_MEMAREA. > + */ > + struct rte_memarea *user_memarea; > Jerin already suggested a union here. I'll add another reason to do so: if in the future there will be new memarea types that require new options, one pointer-sized field can be used to pass anything without breaking the ABI once this structure becomes stable. > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Update memory's refcnt. > + * > + * Update one memory region's refcnt. > + * > + * @param ma > + * The pointer of memarea. > + * @param ptr > + * The pointer of memory region which need be updated refcnt. > + * @param value > + * The value which need be updated. > + * Note: it could be negative. > + * > + * @return > + * 0 on success. Otherwise negative value is returned. > + */ > +__rte_experimental > +int rte_memarea_refcnt_update(struct rte_memarea *ma, void *ptr, int16_t > value); > If this function only updates the refcnt, an API to inspect the refcnt is missing. Furthermore, in this case refcnt is just a value attached to every object, what is the benefit compared to simply storing it in the object? If this function also frees "ptr" when refcnt reaches zero, missing is a way for the user to know that it did. What happens if refcnt > 1 on rte_memarea_free()? I don't think refcnt belongs to this library. A principal objection: memarea is for freeing all objects at once, refcnt is for releasing objects one-by-one when they're not used. Technical issues I foresee: refcnt can be atomic (and require alignment) or not, 16 bits may be too few (rte_flow_action_handle ref'd by thousands of rte_flow). Refcnt could be optional per memarea, but it seems like another complication. --000000000000d47ab405e774b32b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Not= e:
a) the memarea is oriented towards the application layer, whic= h could
provides 'region-based memory management' [1] function.
=C2=A0
Judging from the API, this library would r= ather provide
an interface to a generic allocator over a fixed me= mory extent,
because it offers freeing of specific elements, and = thus must track them.
So it's more than RBMM. Is this intend= ed?
It's a very interesting RFC anyway, just trying to un= derstand the scope.

b) the eal library also provide memory zone/heap management, but these
are tied to huge pages management.
[...]
+ * The memarea is a collection of allocated objects that can be e= fficiently
+ * alloc or free all at once, the main feature are as follows:
+ *=C2=A0 =C2=A0a) it facilitate alloc and free of memory with low overhead= .
+ *=C2=A0=C2=A0 [...]
+ *=C2=A0 =C2=A0c) it supports MT-safe as long as it's specified at cre= ation time.

These two bullets seem to a= dd the most value compared to DPDK heap API.
DPDK heap overhead i= s at least 64 bytes per allocation (sizeof malloc_elem),
so I ass= ume memarea aims at a large number of small elements.
=C2=A0
+struct rte_memarea_param {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0char name[RTE_MEMAREA_NAMESIZE]; /**< Name o= f memarea */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0enum rte_memarea_source source;=C2=A0 /**< M= emory source of memarea */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0uint64_t size;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/**< Size (byte) of memarea */
=

Is it an upper limit or a guaranteed size?
It probably depends on the source: guaranteed f= or USER_ADDR,
upper limit for SYSAPI (or it= would be no different from USER_ADDR),
not= sure about USER_MEMAREA.

Do you envision memarea as always limited?
Ge= neric allocators usually have means of adding extents,
even if this one doesn't currently.

Nit: size is uint64_t here but= uint32_t in rte_memarea_allloc().
Should be size_t in both place= s.
=C2=A0
+=C2=A0 =C2=A0 =C2=A0 =C2=A0uint32_t align;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /**< Align of allocated object */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0/** Indicates whether the memarea should be MT-= safe */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0bool mt_safe;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0/** Indicates whether the memarea is visible to= multiple process.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 * If the memory source is RTE_MEMAREA_SOURCE_U= SER_ADDR, this filed
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 * depends on user settings and must be set. +=C2=A0 =C2=A0 =C2=A0 =C2=A0 * If the memory source is RTE_MEMAREA_SOURCE_S= YSAPI or
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 * RTE_MEMAREA_SOURCE_USER_MEMAREA, this filed = does not need to be set.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0bool mp_visible;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0/** User provided address, this field is valid = only when source
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 * is set to RTE_MEMAREA_SOURCE_USER_ADDR.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0void *user_addr;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0/** User provided memarea, this field is valid = only when source
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 * is set to RTE_MEMAREA_SOURCE_MEMAREA.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0struct rte_memarea *user_memarea;

Jerin already suggested a union here.
I= 9;ll add another reason to do so: if in the future there will be new memare= a types
that require new options, one pointer-sized field can be = used to pass anything
without breaking the ABI once this structur= e becomes stable.
=C2=A0
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Update memory's refcnt.
+ *
+ * Update one memory region's refcnt.
+ *
+ * @param ma
+ *=C2=A0 =C2=A0The pointer of memarea.
+ * @param ptr
+ *=C2=A0 =C2=A0The pointer of memory region which need be updated refcnt.<= br> + * @param value
+ *=C2=A0 =C2=A0The value which need be updated.
+ *=C2=A0 =C2=A0Note: it could be negative.
+ *
+ * @return
+ *=C2=A0 =C2=A00 on success. Otherwise negative value is returned.
+ */
+__rte_experimental
+int rte_memarea_refcnt_update(struct rte_memarea *ma, void *ptr, int16_t v= alue);

If this fu= nction only updates the refcnt, an API to inspect the refcnt is missing.
Furthermore, in this case refcnt is just a value attached to every = object,
what is the benefit compared to simply storing it in the = object?

If this function also frees "ptr"= ; when refcnt reaches zero,
missing is a way for the user to know= that it did.
What happens if refcnt > 1 on rte_memarea_free()= ?

I don't think refcnt belongs to this lib= rary.
A principal objection: memarea is for freeing all objects a= t once,
refcnt is for releasing objects one-by-one when they'= re not used.
Technical issues I foresee: refcnt can be atomic= (and require alignment) or not,
16 bits may be too few (rte_flow= _action_handle ref'd by thousands of rte_flow).
Refcnt could = be optional per memarea, but it seems like another complication.
<= div>
--000000000000d47ab405e774b32b--