Re: [PATCH v8 1/9] memarea: introduce memarea library

[fengchengwen <fengchengwen@huawei.com>]

Hi Dmitry,

On 2022/10/11 23:58, Dmitry Kozlyuk wrote:
> 2022-10-11 12:17 (UTC+0000), Chengwen Feng:
> [...]
>> diff --git a/doc/guides/prog_guide/memarea_lib.rst b/doc/guides/prog_guide/memarea_lib.rst
>> new file mode 100644
>> index 0000000000..85ad57145f
>> --- /dev/null
>> +++ b/doc/guides/prog_guide/memarea_lib.rst
>> @@ -0,0 +1,39 @@
>> +..  SPDX-License-Identifier: BSD-3-Clause
>> +    Copyright(c) 2022 HiSilicon Limited
>> +
>> +Memarea Library
>> +===============
>> +
>> +Introduction
>> +------------
>> +
>> +The memarea library provides an allocator of variable-size objects, it is
>> +oriented towards the application layer, which could provides 'region-based
>> +memory management' function [1].
>> +
>> +The main features are as follows:
>> +
>> +* The allocated object aligned at ``RTE_CACHE_LINE_SIZE`` default.
> 
> Isn't this an implementation detail?
> Stating it in the API description limits optimization opportunities.
> Cache line alignment is good in many cases,
> but it can also be a waste of space,
> e.g. for a thread-unsafe region for small objects.
> Can this limitation only (temporarily?) apply to user memory?
> Or can the minimal alignment be a property of memarea?

You are right, to be more general, I'll add the align parameter to rte_memarea_alloc instead of stating it here.

> 
>> +
>> +* The memory region can be initialized from the following memory sources:
>> +  a) HEAP: e.g. invoke ``rte_malloc_socket``. b) LIBC: e.g. invoke
>> +  posix_memalign to obtain. c) User memory: it can be from e.g. rte_extmem_xxx
>> +  as long as it is available. d) Another memarea: it can be allocated from
>> +  another memarea.
> 
> I think mentioning rte_extmem_xxx() is bogus
> because user memory does not need to be registered with DPDK
> (I understand it's an example, but still an unrelated reference).
> Please format as a list.

okay

> 
>> +
>> +* It provides refcnt feature which could be useful in multi-reader scenario.
>> +
>> +* It supports MT-safe as long as it's specified at creation time.
>> +
>> +Library API Overview
>> +--------------------
>> +
>> +The ``rte_memarea_create()`` function is used to create a memarea, the function
>> +returns the pointer to the created memarea or ``NULL`` if the creation failed.
>> +
>> +The ``rte_memarea_destroy()`` function is used to destroy a memarea.
>> +
>> +Reference
>> +---------
>> +
>> +[1] https://en.wikipedia.org/wiki/Region-based_memory_management
>> diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
>> index 2da8bc9661..f5a67cec7b 100644
>> --- a/doc/guides/rel_notes/release_22_11.rst
>> +++ b/doc/guides/rel_notes/release_22_11.rst
>> @@ -63,6 +63,12 @@ New Features
>>    In theory this implementation should work with any target based on
>>    ``LoongArch`` ISA.
>>  
>> +* **Added memarea library.**
>> +
>> +  The memarea library is an allocator of variable-size objects, it is oriented
>> +  towards the application layer, which could provides 'region-based memory
>> +  management' function.
> 
> "which could provides" -> "providing"

okay

> 
>> +
>>  * **Added support for multiple mbuf pools per ethdev Rx queue.**
>>  
>>    The capability allows application to provide many mempools
> [...]
> .
>