> From: fengchengwen [mailto:fengcheng...@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:fengcheng...@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 <fengcheng...@huawei.com> > >> Reviewed-by: Dongdong Liu <liudongdo...@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 <m...@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.
