fix[kmpp_obj]: Fix grp_cfg and buf_cfg leak in kmpp_obj_test

1. Use mem pool to alloc KmppObjImpl
2. Add kmpp_obj_put_impl to release KmppObjImpl header only.
3. Add grp_cfg and buf_cfg release operation.
4. Use MPP_SINGLETON to init kmpp_venc_cfg.

Signed-off-by: Herman Chen <herman.chen@rock-chips.com>
Change-Id: I865d4d990d7e89598b2f17d85460b809f7f602df
This commit is contained in:
Herman Chen
2025-07-08 10:43:49 +08:00
parent e0e59e5ce2
commit 5497c458fe
7 changed files with 131 additions and 104 deletions

View File

@@ -51,7 +51,10 @@ MppTrie kmpp_objdef_get_trie(KmppObjDef def);
rk_s32 kmpp_obj_get(KmppObj *obj, KmppObjDef def, const char *caller);
rk_s32 kmpp_obj_get_by_name(KmppObj *obj, const char *name, const char *caller);
rk_s32 kmpp_obj_get_by_sptr(KmppObj *obj, KmppShmPtr *sptr, const char *caller);
/* release object and impl head */
rk_s32 kmpp_obj_put(KmppObj obj, const char *caller);
/* release impl head only */
rk_s32 kmpp_obj_put_impl(KmppObj obj, const char *caller);
rk_s32 kmpp_obj_check(KmppObj obj, const char *caller);
rk_s32 kmpp_obj_ioctl(KmppObj obj, rk_s32 cmd, KmppObj in, KmppObj out, const char *caller);
@@ -59,6 +62,7 @@ rk_s32 kmpp_obj_ioctl(KmppObj obj, rk_s32 cmd, KmppObj in, KmppObj out, const ch
#define kmpp_obj_get_by_name_f(obj, name) kmpp_obj_get_by_name(obj, name, __FUNCTION__)
#define kmpp_obj_get_by_sptr_f(obj, sptr) kmpp_obj_get_by_sptr(obj, sptr, __FUNCTION__)
#define kmpp_obj_put_f(obj) kmpp_obj_put(obj, __FUNCTION__)
#define kmpp_obj_put_impl_f(obj) kmpp_obj_put_impl(obj, __FUNCTION__)
#define kmpp_obj_check_f(obj) kmpp_obj_check(obj, __FUNCTION__)
#define kmpp_obj_ioctl_f(obj, cmd, in, out) kmpp_obj_ioctl(obj, cmd, in, out, __FUNCTION__)

View File

@@ -255,8 +255,16 @@ KMPP_OBJ_ENTRY_TABLE(KMPP_OBJ_NAME, VAL_ENTRY_TBL, VAL_ENTRY_TBL,
void CONCAT_US(KMPP_OBJ_NAME, register)(void)
{
rk_s32 impl_size = (sizeof(KMPP_OBJ_IMPL_TYPE) + KMPP_OBJ_EXTRA_SIZE + 3) & ~3;
rk_s32 __flag_base = impl_size << 3;
mpp_env_get_u32(TO_STR(CONCAT_US(KMPP_OBJ_NAME, debug)), &KMPP_OBJ_DEF_DEUBG(KMPP_OBJ_NAME), 0);
KMPP_OBJ_DBG_LOG("register enter\n");
kmpp_objdef_get(&KMPP_OBJ_DEF(KMPP_OBJ_NAME), TO_STR(KMPP_OBJ_INTF_TYPE));
if (KMPP_OBJ_DEF(KMPP_OBJ_NAME)) {
KMPP_OBJ_DBG_LOG(TO_STR(KMPP_OBJ_NAME) " found at kernel\n");
} else {
rk_s32 __entry_size = (sizeof(KMPP_OBJ_IMPL_TYPE) + KMPP_OBJ_EXTRA_SIZE + 3) & ~3;
rk_s32 __flag_base = __entry_size << 3;
rk_s32 __flag_step = 0;
rk_s32 __flag_prev = 0;
rk_s32 __flag_record[ELEM_FLAG_RECORD_MAX];
@@ -265,27 +273,20 @@ void CONCAT_US(KMPP_OBJ_NAME, register)(void)
(void) __flag_prev;
(void) __flag_record;
mpp_env_get_u32(TO_STR(CONCAT_US(KMPP_OBJ_NAME, debug)), &KMPP_OBJ_DEF_DEUBG(KMPP_OBJ_NAME), 0);
kmpp_objdef_get(&KMPP_OBJ_DEF(KMPP_OBJ_NAME), TO_STR(KMPP_OBJ_INTF_TYPE));
if (KMPP_OBJ_DEF(KMPP_OBJ_NAME)) {
KMPP_OBJ_DBG_LOG(TO_STR(KMPP_OBJ_NAME) " found at kernel\n");
return;
}
KMPP_OBJ_DBG_LOG("register enter\n");
kmpp_objdef_register(&KMPP_OBJ_DEF(KMPP_OBJ_NAME), impl_size, TO_STR(KMPP_OBJ_INTF_TYPE));
kmpp_objdef_register(&KMPP_OBJ_DEF(KMPP_OBJ_NAME), __entry_size, TO_STR(KMPP_OBJ_INTF_TYPE));
if (!KMPP_OBJ_DEF(KMPP_OBJ_NAME)) {
mpp_loge_f(TO_STR(KMPP_OBJ_NAME) " init failed\n");
return;
}
KMPP_OBJ_DBG_LOG(TO_STR(KMPP_OBJ_NAME) " registered at userspace\n");
KMPP_OBJ_ENTRY_TABLE(KMPP_OBJ_NAME, ENTRY_TO_TRIE, ENTRY_TO_TRIE,
ENTRY_TO_TRIE, ENTRY_TO_TRIE, ENTRY_TO_TRIE)
kmpp_objdef_add_entry(KMPP_OBJ_DEF(KMPP_OBJ_NAME), NULL, NULL);
KMPP_OBJ_ENTRY_TABLE(KMPP_OBJ_NAME, ENTRY_QUERY, ENTRY_QUERY,
HOOK_QUERY, HOOK_QUERY, ENTRY_NOTHING);
}
#if defined(KMPP_OBJ_FUNC_INIT)
kmpp_objdef_add_init(KMPP_OBJ_DEF(KMPP_OBJ_NAME), KMPP_OBJ_FUNC_INIT);

View File

@@ -130,8 +130,6 @@ typedef struct KmppObjImpl_t {
KmppObjDefImpl *def;
/* trie for fast access */
MppTrie trie;
/* malloc flag */
rk_u32 need_free;
KmppShmPtr *shm;
void *entry;
} KmppObjImpl;
@@ -274,6 +272,11 @@ static void kmpp_objs_deinit(void)
for (i = 0; i < p->count; i++) {
KmppObjDefImpl *impl = &p->defs[i];
if (impl->pool) {
mpp_mem_pool_deinit_f(impl->pool);
impl->pool = NULL;
}
if (impl->trie) {
mpp_trie_deinit(impl->trie);
impl->trie = NULL;
@@ -385,6 +388,11 @@ static void kmpp_objs_init(void)
break;
}
/* create KmppObjImpl header pool for kenel objdef */
impl->pool = mpp_mem_pool_init_f(name, sizeof(KmppObjImpl));
if (!impl->pool)
mpp_loge_f("init mem pool %s size %d failed\n", name, sizeof(KmppObjImpl));
impl->trie = trie_objdef;
info_objdef = mpp_trie_get_info(trie_objdef, "__index");
@@ -392,6 +400,7 @@ static void kmpp_objs_init(void)
info_objdef = mpp_trie_get_info(trie_objdef, "__size");
impl->entry_size = info_objdef ? *(rk_s32 *)mpp_trie_info_ctx(info_objdef) : 0;
impl->name = name;
impl->is_kobj = 1;
info = mpp_trie_get_info_next(trie, info);
obj_dbg_share("%2d:%2d - %s offset %d entry_size %d\n",
@@ -421,14 +430,12 @@ rk_s32 kmpp_objdef_put(KmppObjDef def)
rk_s32 ret = rk_nok;
if (impl) {
rk_s32 release = 0;
if (impl->is_kobj) {
impl->ref_cnt--;
if (!impl->ref_cnt) {
if (impl->trie) {
ret = mpp_trie_deinit(impl->trie);
impl->trie = NULL;
}
}
if (!impl->ref_cnt)
release = 1;
} else {
if (impl->cfg) {
mpp_cfg_put_all(impl->cfg);
@@ -436,16 +443,24 @@ rk_s32 kmpp_objdef_put(KmppObjDef def)
/* When init with MppCfgObj the trie is associated to the MppCfgObj */
impl->trie = NULL;
}
if (impl->trie) {
ret = mpp_trie_deinit(impl->trie);
impl->trie = NULL;
release = 1;
}
if (release) {
if (impl->pool) {
mpp_mem_pool_deinit_f(impl->pool);
impl->pool = NULL;
}
mpp_free(impl);
if (impl->trie) {
mpp_trie_deinit(impl->trie);
impl->trie = NULL;
}
}
if (!impl->is_kobj)
mpp_free(impl);
ret = rk_ok;
}
@@ -481,7 +496,7 @@ rk_s32 kmpp_objdef_register(KmppObjDef *def, rk_s32 size, const char *name)
impl->buf_size = size + sizeof(KmppObjImpl);
impl->ref_cnt = 1;
obj_dbg_flow("kmpp_objdef_register %-16s size %4d - %p\n", name, size, impl);
obj_dbg_flow("objdef %-16s registered size %4d - %p\n", name, size, impl);
*def = impl;
@@ -549,9 +564,10 @@ rk_s32 kmpp_objdef_add_entry(KmppObjDef def, const char *name, KmppEntry *tbl)
obj_dbg_entry("objdef %-16s entry size %4d buf size %4d -> %4d\n", impl->name,
impl->entry_size, old_size, impl->buf_size);
mpp_assert(!impl->pool);
impl->pool = mpp_mem_pool_init_f(impl->name, impl->buf_size);
if (!impl->pool) {
mpp_loge_f("get mem pool size %d failed\n", impl->buf_size);
mpp_loge_f("init mem pool %s size %d failed\n", impl->name, impl->buf_size);
ret = rk_nok;
}
}
@@ -630,7 +646,7 @@ rk_s32 kmpp_objdef_get(KmppObjDef *def, const char *name)
info = mpp_trie_get_info(p->trie, name);
if (!info) {
obj_dbg_flow("failed to get kernel objdef %s\n", name);
obj_dbg_flow("objdef %-16s can not be found in kernel \n", name);
return rk_nok;
}
@@ -638,13 +654,12 @@ rk_s32 kmpp_objdef_get(KmppObjDef *def, const char *name)
KmppObjDefImpl *impl = &p->defs[info->index];
impl->ref_cnt++;
impl->is_kobj = 1;
*def = impl;
return rk_ok;
}
mpp_loge_f("failed to get objdef %s index %d max %d\n",
mpp_loge_f("objdef %-16s is found but with invalid index %d max %d\n",
name, info->index, p->count);
return rk_nok;
@@ -792,16 +807,13 @@ rk_s32 kmpp_obj_get(KmppObj *obj, KmppObjDef def, const char *caller)
def_impl = (KmppObjDefImpl *)def;
/* use buf_size to check userspace objdef or kernel objdef */
mpp_assert(def_impl->pool);
/* userspace objdef path */
if (def_impl->buf_size) {
if (def_impl->pool)
impl = mpp_mem_pool_get(def_impl->pool, caller);
else
impl = mpp_calloc_size(KmppObjImpl, def_impl->buf_size);
if (!impl) {
mpp_loge_f("malloc obj %s impl %d failed at %s\n",
mpp_loge_f("get obj %s impl %d failed at %s\n",
def_impl->name, def_impl->buf_size, caller);
return ret;
}
@@ -809,7 +821,6 @@ rk_s32 kmpp_obj_get(KmppObj *obj, KmppObjDef def, const char *caller)
impl->name = def_impl->name;
impl->def = def;
impl->trie = def_impl->trie;
impl->need_free = 1;
impl->shm = NULL;
impl->entry = (void *)(impl + 1);
@@ -826,9 +837,9 @@ rk_s32 kmpp_obj_get(KmppObj *obj, KmppObjDef def, const char *caller)
if (!p)
return ret;
impl = mpp_calloc(KmppObjImpl, 1);
impl = (KmppObjImpl *)mpp_mem_pool_get(def_impl->pool, caller);
if (!impl) {
mpp_loge_f("malloc obj impl %d failed at %s\n", sizeof(KmppObjImpl), caller);
mpp_loge_f("get obj impl %d failed at %s\n", sizeof(KmppObjImpl), caller);
return ret;
}
@@ -850,11 +861,10 @@ rk_s32 kmpp_obj_get(KmppObj *obj, KmppObjDef def, const char *caller)
impl->name = def_impl->name;
impl->def = def;
impl->trie = def_impl->trie;
impl->need_free = 1;
impl->shm = U64_TO_PTR(uaddr);
impl->entry = U64_TO_PTR(uaddr + p->entry_offset);
obj_dbg_flow("get obj %s - %p entry [u:k] %llx:%llx at %s\n", def_impl->name,
obj_dbg_flow("get obj %-16s - %p entry [u:k] %llx:%llx at %s\n", def_impl->name,
impl, uaddr, ioc->obj_sptr[0].kaddr, caller);
/* write userspace object address to share memory userspace private value */
@@ -883,7 +893,7 @@ rk_s32 kmpp_obj_get_by_name(KmppObj *obj, const char *name, const char *caller)
info = mpp_trie_get_info(p->trie, name);
if (!info) {
mpp_loge_f("failed to get objdef %s at %s\n", name, caller);
obj_dbg_flow("objdef %-16s can not be found in kernel \n", name);
return rk_nok;
}
@@ -894,8 +904,8 @@ rk_s32 kmpp_obj_get_by_name(KmppObj *obj, const char *name, const char *caller)
return kmpp_obj_get(obj, impl, caller);
}
mpp_loge_f("failed to get objdef %s index %d max %d at %s\n",
name, info->index, p->count, caller);
mpp_loge_f("objdef %-16s is found but with invalid index %d max %d\n",
name, info->index, p->count);
return rk_nok;
}
@@ -936,20 +946,20 @@ rk_s32 kmpp_obj_get_by_sptr(KmppObj *obj, KmppShmPtr *sptr, const char *caller)
}
}
impl = mpp_calloc(KmppObjImpl, 1);
mpp_assert(def->pool);
impl = (KmppObjImpl *)mpp_mem_pool_get(def->pool, caller);
if (!impl) {
mpp_loge_f("malloc obj impl %d failed at %s\n", sizeof(KmppObjImpl), caller);
mpp_loge_f("get obj impl %d failed at %s\n", sizeof(KmppObjImpl), caller);
return ret;
}
impl->name = def->name;
impl->def = def;
impl->trie = def->trie;
impl->need_free = 1;
impl->shm = (KmppShmPtr *)uptr;
impl->entry = uptr + p->entry_offset;
obj_dbg_flow("get obj %s - %p by sptr [u:k] %llx:%llx at %s\n", def->name,
obj_dbg_flow("get obj %-16s - %p by sptr [u:k] %llx:%llx at %s\n", def->name,
impl, sptr->uaddr, sptr->kaddr, caller);
/* write userspace object address to share memory userspace private value */
@@ -964,22 +974,22 @@ rk_s32 kmpp_obj_put(KmppObj obj, const char *caller)
{
if (obj) {
KmppObjImpl *impl = (KmppObjImpl *)obj;
KmppObjDefImpl *def = impl->def;
KmppObjs *p;
/* use shm to check userspace objdef or kernel objdef */
/* userspace objdef path */
if (!impl->shm) {
if (impl->def) {
KmppObjDefImpl *def = impl->def;
mpp_assert(def);
if (def) {
if (def->deinit)
def->deinit(impl->entry, caller);
if (def->pool) {
mpp_assert(def->pool);
mpp_mem_pool_put(def->pool, impl, caller);
return rk_ok;
}
}
mpp_free(impl);
return rk_ok;
@@ -988,7 +998,6 @@ rk_s32 kmpp_obj_put(KmppObj obj, const char *caller)
p = get_objs(caller);
if (p && p->fd >= 0) {
KmppObjIocArg *ioc = alloca(sizeof(KmppObjIocArg) + sizeof(KmppShmPtr));
KmppObjDefImpl *def = impl->def;
rk_s32 ret;
ioc->count = 1;
@@ -996,7 +1005,7 @@ rk_s32 kmpp_obj_put(KmppObj obj, const char *caller)
ioc->obj_sptr[0].uaddr = impl->shm->uaddr;
ioc->obj_sptr[0].kaddr = impl->shm->kaddr;
obj_dbg_flow("put obj %s - %p entry [u:k] %llx:%llx at %s\n", def ? def->name : NULL,
obj_dbg_flow("put obj %-16s - %p entry [u:k] %llx:%llx at %s\n", def ? def->name : NULL,
impl, impl->shm->uaddr, impl->shm->kaddr, caller);
ret = ioctl(p->fd, KMPP_SHM_IOC_PUT_SHM, ioc);
@@ -1005,8 +1014,8 @@ rk_s32 kmpp_obj_put(KmppObj obj, const char *caller)
}
impl->shm = NULL;
if (impl->need_free)
mpp_free(impl);
mpp_assert(def->pool);
mpp_mem_pool_put(def->pool, impl, caller);
return rk_ok;
}
@@ -1014,6 +1023,28 @@ rk_s32 kmpp_obj_put(KmppObj obj, const char *caller)
return rk_nok;
}
rk_s32 kmpp_obj_put_impl(KmppObj obj, const char *caller)
{
if (obj) {
KmppObjImpl *impl = (KmppObjImpl *)obj;
KmppObjDefImpl *def = impl->def;
mpp_assert(def);
if (def) {
if (def->deinit)
def->deinit(impl->entry, caller);
mpp_assert(def->pool);
mpp_mem_pool_put(def->pool, impl, caller);
return rk_ok;
}
}
return rk_nok;
}
rk_s32 kmpp_obj_check(KmppObj obj, const char *caller)
{
KmppObjImpl *impl = (KmppObjImpl *)obj;

View File

@@ -12,6 +12,7 @@
#include "mpp_mem.h"
#include "mpp_debug.h"
#include "mpp_common.h"
#include "mpp_singleton.h"
#include "kmpp_obj.h"
#include "rk_venc_kcfg.h"
@@ -38,36 +39,30 @@ static char *kcfg_names[] = {
[MPP_VENC_KCFG_TYPE_STOP] = "KmppVencStopCfg",
};
static KmppObjDef kcfg_defs[MPP_VENC_KCFG_TYPE_BUTT] = {NULL};
static pthread_mutex_t lock;
static void mpp_venc_kcfg_def_init() __attribute__((constructor));
static void mpp_venc_kcfg_def_deinit() __attribute__((destructor));
static void mpp_venc_kcfg_def_init(void)
{
pthread_mutexattr_t attr;
RK_U32 i;
pthread_mutexattr_init(&attr);
pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
pthread_mutex_init(&lock, &attr);
pthread_mutexattr_destroy(&attr);
for (i = 0; i < MPP_VENC_KCFG_TYPE_BUTT; i++) {
kmpp_objdef_get(&kcfg_defs[i], kcfg_names[i]);
}
}
static void mpp_venc_kcfg_def_deinit(void)
{
RK_U32 i;
pthread_mutex_lock(&lock);
for (i = 0; i < MPP_VENC_KCFG_TYPE_BUTT; i++) {
if (kcfg_defs[i]) {
kmpp_objdef_put(kcfg_defs[i]);
kcfg_defs[i] = NULL;
}
}
pthread_mutex_unlock(&lock);
pthread_mutex_destroy(&lock);
}
MPP_SINGLETON(MPP_SGLN_KMPP_VENC_CFG, kmpp_venc_cfg, mpp_venc_kcfg_def_init, mpp_venc_kcfg_def_deinit)
MPP_RET mpp_venc_kcfg_init(MppVencKcfg *cfg, MppVencKcfgType type)
{
KmppObj obj = NULL;
@@ -84,18 +79,7 @@ MPP_RET mpp_venc_kcfg_init(MppVencKcfg *cfg, MppVencKcfgType type)
mpp_env_get_u32("venc_kcfg_debug", &venc_kcfg_debug, 0);
pthread_mutex_lock(&lock);
if (kcfg_defs[type] == NULL) {
MPP_RET ret = (MPP_RET)kmpp_objdef_get(&kcfg_defs[type], kcfg_names[type]);
if (ret) {
mpp_err_f("failed to get %s obj def %d\n", kcfg_names[type], type);
pthread_mutex_unlock(&lock);
return MPP_NOK;
}
}
pthread_mutex_unlock(&lock);
if (kcfg_defs[type])
kmpp_obj_get_f(&obj, kcfg_defs[type]);
*cfg = obj;

View File

@@ -236,9 +236,15 @@ done:
if (grp)
kmpp_obj_put_f(grp);
if (grp_cfg)
kmpp_obj_put_impl_f(grp_cfg);
if (buf)
kmpp_obj_put_f(buf);
if (buf_cfg)
kmpp_obj_put_impl_f(buf_cfg);
return ret;
}

View File

@@ -28,6 +28,7 @@ typedef enum MppSingletonId_e {
MPP_SGLN_KMPP_META,
MPP_SGLN_KMPP_FRAME,
MPP_SGLN_KMPP_PACKET,
MPP_SGLN_KMPP_VENC_CFG,
/* userspace base module */
MPP_SGLN_BUFFER,
MPP_SGLN_META,

View File

@@ -100,12 +100,12 @@ static void mem_pool_srv_init()
INIT_LIST_HEAD(&srv->list);
}
static void put_pool(MppMemPoolSrv *srv, MppMemPoolImpl *impl)
static void put_pool(MppMemPoolSrv *srv, MppMemPoolImpl *impl, const char *caller)
{
MppMemPoolNode *node, *m;
if (impl != impl->check) {
mpp_err_f("invalid mem impl %p check %p\n", impl, impl->check);
mpp_err_f("invalid mem impl %p check %p at %s\n", impl, impl->check, caller);
return;
}
@@ -122,8 +122,8 @@ static void put_pool(MppMemPoolSrv *srv, MppMemPoolImpl *impl)
}
if (!list_empty(&impl->used)) {
mpp_err_f("pool %-16s found %d used buffer size %4d\n",
impl->name, impl->used_count, impl->size);
mpp_err_f("pool %-16s found %d used buffer size %4d at %s\n",
impl->name, impl->used_count, impl->size, caller);
list_for_each_entry_safe(node, m, &impl->used, MppMemPoolNode, list) {
MPP_FREE(node);
@@ -132,8 +132,8 @@ static void put_pool(MppMemPoolSrv *srv, MppMemPoolImpl *impl)
}
if (impl->used_count || impl->unused_count)
mpp_err_f("pool %-16s size %4d found leaked buffer used:unused [%d:%d]\n",
impl->name, impl->size, impl->used_count, impl->unused_count);
mpp_err_f("pool %-16s size %4d found leaked buffer used:unused [%d:%d] at %s\n",
impl->name, impl->size, impl->used_count, impl->unused_count, caller);
pthread_mutex_unlock(&impl->lock);
@@ -159,7 +159,7 @@ static void mem_pool_srv_deinit()
list_for_each_entry_safe(pos, n, &srv->list, MppMemPoolImpl, service_link) {
mem_pool_dbg_exit("pool %-16s size %4d leaked\n", pos->name, pos->size);
put_pool(srv, pos);
put_pool(srv, pos, __FUNCTION__);
}
}
@@ -218,7 +218,7 @@ void mpp_mem_pool_deinit(MppMemPool pool, const char *caller)
mem_pool_dbg_flow("pool %-16s size %4d deinit at %s\n",
impl->name, impl->size, caller);
put_pool(srv, impl);
put_pool(srv, impl, caller);
}
void *mpp_mem_pool_get(MppMemPool pool, const char *caller)