From f2e65c02e69887c2dc70551772daefb1602e3e2b Mon Sep 17 00:00:00 2001 From: "Chen, Phoebe" Date: Wed, 12 Feb 2025 16:48:17 -0500 Subject: [PATCH] amd/vpelib: Fix memory leak from segment ctx [WHY] There is potential memory leak from vpe_alloc_segment_ctx. This memory leak occurs only in multi-frame VPE tests where between vpe_create and vpe_destroy, multiple calls are made to vpe_check_support that allocates new segment context without releasing the old one. [HOW] Allocate segment_ctx only when it is not already allocated. If it is already allocated, check whether re-allocation is needed. If not, skip the allocation. Signed-off-by: Phoebe Chen Reviewed-by: Roy Chan Reviewed-by: Jesse Agate Acked-by: ChuanYu Tseng Part-of: --- .../vpelib/src/chip/vpe10/vpe10_resource.c | 17 ++++++------ .../vpelib/src/chip/vpe11/vpe11_resource.c | 17 ++++++------ src/amd/vpelib/src/core/inc/resource.h | 3 ++- src/amd/vpelib/src/core/resource.c | 26 ++++++++++++++----- 4 files changed, 39 insertions(+), 24 deletions(-) diff --git a/src/amd/vpelib/src/chip/vpe10/vpe10_resource.c b/src/amd/vpelib/src/chip/vpe10/vpe10_resource.c index dae3b5a3f18..be0947b7d1f 100644 --- a/src/amd/vpelib/src/chip/vpe10/vpe10_resource.c +++ b/src/amd/vpelib/src/chip/vpe10/vpe10_resource.c @@ -249,9 +249,10 @@ enum vpe_status vpe10_set_num_segments(struct vpe_priv *vpe_priv, struct stream_ struct scaler_data *scl_data, struct vpe_rect *src_rect, struct vpe_rect *dst_rect, uint32_t *max_seg_width, uint32_t recout_width_alignment) { - uint16_t num_segs; - struct dpp *dpp = vpe_priv->resource.dpp[0]; - const uint32_t max_lb_size = dpp->funcs->get_line_buffer_size(); + uint16_t num_segs; + struct dpp *dpp = vpe_priv->resource.dpp[0]; + const uint32_t max_lb_size = dpp->funcs->get_line_buffer_size(); + enum vpe_status res = VPE_STATUS_OK; (void)recout_width_alignment; @@ -259,13 +260,13 @@ enum vpe_status vpe10_set_num_segments(struct vpe_priv *vpe_priv, struct stream_ num_segs = vpe_get_num_segments(vpe_priv, src_rect, dst_rect, *max_seg_width); - stream_ctx->segment_ctx = vpe_alloc_segment_ctx(vpe_priv, num_segs); - if (!stream_ctx->segment_ctx) - return VPE_STATUS_NO_MEMORY; + res = vpe_alloc_segment_ctx(vpe_priv, stream_ctx, num_segs); - stream_ctx->num_segments = num_segs; + if (res == VPE_STATUS_OK) { + stream_ctx->num_segments = num_segs; + } - return VPE_STATUS_OK; + return res; } bool vpe10_get_dcc_compression_output_cap(const struct vpe *vpe, const struct vpe_dcc_surface_param *params, struct vpe_surface_dcc_cap *cap) diff --git a/src/amd/vpelib/src/chip/vpe11/vpe11_resource.c b/src/amd/vpelib/src/chip/vpe11/vpe11_resource.c index b964a241150..2c02f134829 100644 --- a/src/amd/vpelib/src/chip/vpe11/vpe11_resource.c +++ b/src/amd/vpelib/src/chip/vpe11/vpe11_resource.c @@ -237,9 +237,10 @@ enum vpe_status vpe11_set_num_segments(struct vpe_priv *vpe_priv, struct stream_ struct scaler_data *scl_data, struct vpe_rect *src_rect, struct vpe_rect *dst_rect, uint32_t *max_seg_width, uint32_t recout_width_alignment) { - uint16_t num_segs; - struct dpp *dpp = vpe_priv->resource.dpp[0]; - const uint32_t max_lb_size = dpp->funcs->get_line_buffer_size(); + uint16_t num_segs; + struct dpp *dpp = vpe_priv->resource.dpp[0]; + const uint32_t max_lb_size = dpp->funcs->get_line_buffer_size(); + enum vpe_status res = VPE_STATUS_OK; (void)recout_width_alignment; *max_seg_width = min(*max_seg_width, max_lb_size / scl_data->taps.v_taps); @@ -250,13 +251,13 @@ enum vpe_status vpe11_set_num_segments(struct vpe_priv *vpe_priv, struct stream_ num_segs += (vpe_priv->vpe_num_instance - (num_segs % vpe_priv->vpe_num_instance)); } - stream_ctx->segment_ctx = vpe_alloc_segment_ctx(vpe_priv, num_segs); - if (!stream_ctx->segment_ctx) - return VPE_STATUS_NO_MEMORY; + res = vpe_alloc_segment_ctx(vpe_priv, stream_ctx, num_segs); - stream_ctx->num_segments = num_segs; + if (res == VPE_STATUS_OK) { + stream_ctx->num_segments = num_segs; + } - return VPE_STATUS_OK; + return res; } bool vpe11_validate_cached_param(struct vpe_priv *vpe_priv, const struct vpe_build_param *param) diff --git a/src/amd/vpelib/src/core/inc/resource.h b/src/amd/vpelib/src/core/inc/resource.h index 21f8a52f5f9..ac658f151f8 100644 --- a/src/amd/vpelib/src/core/inc/resource.h +++ b/src/amd/vpelib/src/core/inc/resource.h @@ -133,7 +133,8 @@ enum vpe_status vpe_construct_resource( void vpe_destroy_resource(struct vpe_priv *vpe_priv, struct resource *res); /** alloc segment ctx*/ -struct segment_ctx *vpe_alloc_segment_ctx(struct vpe_priv *vpe_priv, uint16_t num_segments); +enum vpe_status vpe_alloc_segment_ctx( + struct vpe_priv *vpe_priv, struct stream_ctx *stream_ctx, uint16_t num_segments); /** stream ctx */ struct stream_ctx *vpe_alloc_stream_ctx(struct vpe_priv *vpe_priv, uint32_t num_streams); diff --git a/src/amd/vpelib/src/core/resource.c b/src/amd/vpelib/src/core/resource.c index f4a29fc6bbb..ca70d2d1800 100644 --- a/src/amd/vpelib/src/core/resource.c +++ b/src/amd/vpelib/src/core/resource.c @@ -136,16 +136,28 @@ void vpe_destroy_resource(struct vpe_priv *vpe_priv, struct resource *res) } } -struct segment_ctx *vpe_alloc_segment_ctx(struct vpe_priv *vpe_priv, uint16_t num_segments) +enum vpe_status vpe_alloc_segment_ctx( + struct vpe_priv *vpe_priv, struct stream_ctx *stream_ctx, uint16_t num_segments) { - struct segment_ctx *segment_ctx_base; + // If segment_ctx is already allocated, check if re-allocation needed + if (stream_ctx->segment_ctx) { + if (num_segments != stream_ctx->num_segments) { + // Need to re-allocate segment_ctx. Free it first + vpe_free(stream_ctx->segment_ctx); + stream_ctx->segment_ctx = NULL; + } else { + // No need for re-allocation. Return + return VPE_STATUS_OK; + } + } - segment_ctx_base = (struct segment_ctx *)vpe_zalloc(sizeof(struct segment_ctx) * num_segments); + stream_ctx->segment_ctx = + (struct segment_ctx *)vpe_zalloc(sizeof(struct segment_ctx) * num_segments); + if (!stream_ctx->segment_ctx) { + return VPE_STATUS_NO_MEMORY; + } - if (!segment_ctx_base) - return NULL; - - return segment_ctx_base; + return VPE_STATUS_OK; } static enum vpe_status create_input_config_vector(struct stream_ctx *stream_ctx)