On 4/29/25 00:49, Mathieu Poirier wrote:
CAUTION: This email comes from a non Wind River email account! Do not click links or open attachments unless you recognize the sender and know the content is safe. Hi Xiaolei, On Sat, Apr 26, 2025 at 02:53:47PM +0800, Xiaolei Wang wrote:When rproc->state = RPROC_DETACHED and rproc_attach() is used to attach to the remote processor, if rproc_handle_resources() returns a failure, the resources allocated by rproc_prepare_device() should be released, otherwise the following memory leak will occur. Therefore, add imx_rproc_unprepare() to imx_rproc to release the memory allocated in imx_rproc_prepare(). unreferenced object 0xffff0000861c5d00 (size 128): comm "kworker/u12:3", pid 59, jiffies 4294893509 (age 149.220s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 02 88 00 00 00 00 00 00 10 00 00 00 00 00 ............ backtrace: [<00000000f949fe18>] slab_post_alloc_hook+0x98/0x37c [<00000000adbfb3e7>] __kmem_cache_alloc_node+0x138/0x2e0 [<00000000521c0345>] kmalloc_trace+0x40/0x158 [<000000004e330a49>] rproc_mem_entry_init+0x60/0xf8 [<000000002815755e>] imx_rproc_prepare+0xe0/0x180 [<0000000003f61b4e>] rproc_boot+0x2ec/0x528 [<00000000e7e994ac>] rproc_add+0x124/0x17c [<0000000048594076>] imx_rproc_probe+0x4ec/0x5d4 [<00000000efc298a1>] platform_probe+0x68/0xd8 [<00000000110be6fe>] really_probe+0x110/0x27c [<00000000e245c0ae>] __driver_probe_device+0x78/0x12c [<00000000f61f6f5e>] driver_probe_device+0x3c/0x118 [<00000000a7874938>] __device_attach_driver+0xb8/0xf8 [<0000000065319e69>] bus_for_each_drv+0x84/0xe4 [<00000000db3eb243>] __device_attach+0xfc/0x18c [<0000000072e4e1a4>] device_initial_probe+0x14/0x20 Signed-off-by: Xiaolei Wang <xiaolei.w...@windriver.com> --- drivers/remoteproc/imx_rproc.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c index 74299af1d7f1..c489bd15ee91 100644 --- a/drivers/remoteproc/imx_rproc.c +++ b/drivers/remoteproc/imx_rproc.c @@ -595,6 +595,19 @@ static int imx_rproc_prepare(struct rproc *rproc) return 0; } +static int imx_rproc_unprepare(struct rproc *rproc) +{ + struct rproc_mem_entry *entry, *tmp; + + rproc_coredump_cleanup(rproc); + /* clean up carveout allocations */ + list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) { + list_del(&entry->node); + kfree(entry); + } + return 0; +} +I have reflected long and hard on this. The problem with the above approach is that we do almost the same thing in imx_rproc_prepare() and rproc_resource_cleanup(). Function rproc_resource_cleanup() is able to deal with empty lists so I think it is better to fix the "goto" statements in rproc_attach(). More specifically, replace the "unprepare_device" goto statement with "clean_up_resources" and get rid of the "unprepare_device" label. Please see if that works on your side. I am good with 2/2 of this series.
Thanks to Mathieu for the suggestion, I will try this approach, and also thanks to Peng for the more detailed explanation thanks xiaolei
Thanks, Mathieustatic int imx_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw) { int ret; @@ -675,6 +688,7 @@ imx_rproc_elf_find_loaded_rsc_table(struct rproc *rproc, const struct firmware * static const struct rproc_ops imx_rproc_ops = { .prepare = imx_rproc_prepare, + .unprepare = imx_rproc_unprepare, .attach = imx_rproc_attach, .detach = imx_rproc_detach, .start = imx_rproc_start, -- 2.25.1
