lib_manager: modules: Fix missing call to modules_free
This PR addresses an issue with non-iadk loadable modules for which the modules_free function is not called.
For some of following commits I already prepared PRs - pending review.
- [ ] https://github.com/thesofproject/sof/pull/8897: d37d68961 modules: Remove unused module_entry_point from module_data 87f5e3e69 module_adapter: modules: Remove unused sys_service pointer c951d57d6 module/base: modules: Remove module_adapter from module_data c85d87216 modules: Remove unused buffers 2b49f997e modules: Remove unnecessary functions from Processing Module Adapter
Actual PR content: ae47140d2 lib_manager: modules: move native lib support to lib_manager
-
[ ] https://github.com/thesofproject/sof/pull/8974: 94b9976df lib_manager: Rename lib_manager_get_library_module_desc function ca42dff5a lib_manager: Add lib_manager_get_module_manifest function 11d8a423b ipc4/helper: ipc4_get_comp_drv: Change parameter type to uint32_t
-
[x] https://github.com/thesofproject/sof/pull/8898:
efd957d51 llext_manager: Remove unused desc parameter 7407ecdda lib_manager: llext_manager: Add const to module allocate functions 66f35c92a FIX lib_manager: llext_manager: Add const modifier to module manifest pointers 61e877f15 lib_manager: llext_manager: Add const modifier to module manifest pointers 30fd9df1d lib_manager: llext_manager: Simplifying parameter list for free module 5c6c7c8c2 lib_manager: Simplifying parameter list for lib_manager_register_module 0e05cefe4 lib_manager: Verify preload_page_count from module manifest da99651a8 lib_manager: Add const to library manifest variable -
[x] https://github.com/thesofproject/sof/pull/8905:
3d34605ab modules: lib_manager: Move declare_dynamic_module_adapter to lib_manager 6e80e8bff component: module_adapter: Move module_interface pointer to comp_driver
@lyakh Can you check do it still breaks llext?
@lyakh Can you check do it still breaks llext?
@softwarecki why do you think it's fixed now? What's been changed compared to that version? I compared and I don't see any changes that would fix it. buildinfo is still lost so the module entry point isn't identified and called correctly, AFAICS
@softwarecki why do you think it's fixed now? What's been changed compared to that version? I compared and I don't see any changes that would fix it.
buildinfois still lost so the module entry point isn't identified and called correctly, AFAICS
@lyakh The module entry is taken from a module manifest. This code path has not been changed (lib_manager_allocate_module -> llext_manager_allocate_module -> llext_manager_load_module).
@softwarecki why do you think it's fixed now? What's been changed compared to that version? I compared and I don't see any changes that would fix it.
buildinfois still lost so the module entry point isn't identified and called correctly, AFAICS@lyakh The module entry is taken from a module manifest. This code path has not been changed (
lib_manager_allocate_module->llext_manager_allocate_module->llext_manager_load_module).
@softwarecki I'm talking about buildinfo not the manifest
@softwarecki I'm talking about
buildinfonot the manifest
@softwarecki ...and sorry for a delay - that comment was sitting in my browser for days, I must've forgotten to press "comment" :-(
@lyakh The buildinfo structure is not propagated up because there is no need for it. For llext modules it is retrieved in the llext_manager_link function and checked in llext_manager_allocate_module (see: https://github.com/thesofproject/sof/pull/8935/files#diff-105bb3747dadc76f4ae373fd79ca6aba210883d29b7caee205f4adda6f6d6dccR256) . For other modules types, obtaining and checking is done in lib_manager_register_module (https://github.com/thesofproject/sof/pull/8935/files#diff-957336c9ebdbe16dab94f6e49620e9b112d3b373d3de4d1035cdf289f745a00dR666).
@lyakh The
buildinfostructure is not propagated up because there is no need for it. For llext modules it is retrieved in thellext_manager_linkfunction and checked inllext_manager_allocate_module(see: https://github.com/thesofproject/sof/pull/8935/files#diff-105bb3747dadc76f4ae373fd79ca6aba210883d29b7caee205f4adda6f6d6dccR256) . For other modules types, obtaining and checking is done inlib_manager_register_module(https://github.com/thesofproject/sof/pull/8935/files#diff-957336c9ebdbe16dab94f6e49620e9b112d3b373d3de4d1035cdf289f745a00dR666).
@softwarecki ah, you're right, now I get it! And tested - worked in a short test. It looks good in general, but a double-use test just has caused a DSP panic with this PR - investigating, removing the approval for now
@softwarecki please merge this diff with your "lib_manager: modules: move native lib support to lib_manager" (and also add a description, it certainly deserves one) 20240403-1-sof-ldmod.gitdiff.txt then I'll be able to restore my approval
Rebased and applied @lyakh patch.
@softwarecki @kv2019i @lgirdwood can we merge this one instead of #8974 - the first 3 commits are identical and the fourth one looks good to me too
Sorry @lyakh , I was too quick with https://github.com/thesofproject/sof/pull/8974. @softwarecki can you update this and we can merge this as well?
@softwarecki @wszypelt good to merge or real issue reported by CI ?
@lgirdwood I'm afraid that the PR may contain a sporadic issue, I'm taking this PR for full scope tests, I should have the results tomorrow :)
@lgirdwood @softwarecki After careful checking, I don't see any problems, CI also passed correctly