[PATCH v12 1/6] memarea: introduce memarea library

[Morten Brørup]

> From: fengchengwen [mailto:fengchengwen at huawei.com]
> Sent: Friday, 20 January 2023 09.21
> 
> Hi Morten,
> 
> On 2023/1/15 15:58, Morten Brørup wrote:
> >> From: Chengwen Feng [mailto:fengchengwen at huawei.com]
> >> Sent: Saturday, 14 January 2023 12.50
> >>
> >> The memarea library is an allocator of variable-size object which
> based
> >> on a memory region.
> >>
> >> This patch provides rte_memarea_create() and rte_memarea_destroy()
> API.
> >>
> >> Signed-off-by: Chengwen Feng <fengchengwen at huawei.com>
> >> Reviewed-by: Dongdong Liu <liudongdong3 at huawei.com>
> >> ---
> >
> > [...]
> >
> >> +struct memarea_obj {
> >> +	TAILQ_ENTRY(memarea_obj) obj_node;
> >> +	TAILQ_ENTRY(memarea_obj) free_node;
> >> +	size_t                   size;
> >> +	size_t                   alloc_size;
> >> +	uint32_t                 magic;
> >> +};
> >
> > The magic cookie is for debug purposes only, and should be enclosed
> by #ifdef RTE_LIBRTE_MEMAREA_DEBUG, like in the mempool library [1].

It was just one example; the mbuf library also has RTE_LIBRTE_MBUF_DEBUG.

> 
> In the mempool the cookie mechanism is under debug macro, the main
> reason should be performance considerations.

Yes, the main reason is performance (of production builds).

The secondary reason is pollution of the generated code - unnecessary bloat makes it harder reading the assembly output when debugging, because the assembly output is not clean, but full of irrelevant runtime checks.

> 
> And community recommends that compilation macro be used as little as
> possible.

Yes, but I don't think this recommendation applies to debug code.

I strongly oppose to having run-time bug detection code, such as checking magic cookies, in production builds. It is a well established "best practice" to be able to build software projects for debug or production, and the DPDK project should not deviate from this best practice!

> 
> So I think we could add the following new API like:
> 
> /*
>  * Enable audit in alloc/free/audit.
>  */
> rte_memarea_audit_enable(struct rte_memarea *ma)
> 
> /*
>  * Disable audit in alloc/free/audit.
>  */
> rte_memarea_audit_disable(struct rte_memarea *ma)
> 
> /*
>  * if ptr is NULL, then audit the all objects.
>  * else, only audit the object which the ptr pointers.
>  * if audit fail, will return an error code, and the error log will
> emit.
>  *
>  * The audit is performed only when the audit function is enabled for
> the memarea.
>  */
> int rte_memarea_audit_object(struct rte_memarea *ma, void *ptr)
> 
> 
> So for an application, it can invoke rte_memarea_audit() in its code
> without macro control.
> If it considers that memory corruption may occur in a certain scenario,
> the audit function can be enabled, so could find it by error log or
> retcode.
> 
> This method causes slight performance loss. But it's guaranteed that
> there's no need to recompile, and a binary can be done.

It still pollutes the generated code, making debugging by hand (i.e. reading assembly output) more difficult.

> 
> >
> > [1]:
> https://elixir.bootlin.com/dpdk/latest/source/lib/mempool/rte_mempool.h
> #L154
> >
> > With that added:
> >
> > Series-acked-by: Morten Brørup <mb at smartsharesystems.com>

If you think that this library's cookie checking is important for typical users, you can leave RTE_LIBRTE_MEMAREA_DEBUG enabled by default.

But it must be possible to completely omit such run-time debug checks when building for production.