Add memory usage scan possibility #3389

ktrzcinx · 2020-09-04T06:51:22Z

Such an mechanism may be used to monitor memory utilization and memory leaks.

Possible memory leaks tracking was main motivation for this mechanism. To scan them, memory usage should be scanned before topology creation, then some functional test should be done, topology deletion and iterate whole process a few times - when memory usage grows, then some memory leak is present. To efficiently track memory leaks, memory utilization must be scanned at strictly defined moments in FW life-cycle, it's why scan should be done on host request instead regular memory utilization printing (eq. in trace system).

kernel part: thesofproject/linux#2459

kv2019i

note: SW review as per "RFC" step process in https://thesofproject.github.io/latest/contribute/process/abiprocess.html#sof-abi-changes

Looks like a welcome change, but some comments on the naming and general question whether tihs exposes too much specific implementation information.

src/include/ipc/debug.h

src/include/ipc/header.h

src/include/ipc/debug.h

src/ipc/handler.c

marc-hb · 2020-09-04T19:27:50Z

SOFCI TEST

EDIT:

CML devices were broken because of an internal script change merged 11 hours ago. Fixed by @fredoh9 just now.
Internal Intel CI System/merge/build is down for the WE for maintenance.

lgirdwood · 2020-09-07T09:53:52Z

Jenkins CML CI passing now.

Such a function may be used to monitor memory leaks and system utilization. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

Function name should correspond IPC name. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

lgirdwood · 2020-09-25T10:14:56Z

@ktrzcinx @kv2019i any updates on how kernel will expose this info to userspace ?

plbossart · 2020-09-25T13:29:45Z

@ktrzcinx @kv2019i any updates on how kernel will expose this info to userspace ?

thesofproject/linux#2459

kv2019i

Discussion on the kernel PR is continuing, but it's around the userspace<>kernel interface. It would seem we have no open concerns on the IPC interface change itself. Thus with my ABI hat on, +1 for the change.

plbossart

sounds good at the firmware level, for the kernel I have concerns that we need to prevent pm_runtime from kicking in, otherwise during tests we'll lose all the information. It's more a usability concern than with retrieving information from firmware.

lgirdwood

Can you make this a Kconfig menu entry with a flag in the manifest so kernel knows whether feature is built in.

ktrzcinx · 2020-09-29T13:51:00Z

Added Kconfig option and flag in ext_man_config_data.

lgirdwood · 2020-09-29T14:04:12Z

Seeing a build error

 27%] Building ASM object src/arch/xtensa/hal/CMakeFiles/hal.dir/syscache_asm.S.o
/home/test/jenkins/workspace/sof_prs/src/init/ext_manifest.c:95: error: â€˜CONFIG_DEBUG_MEMORY_USAGE_SCANâ€™ undeclared here (not in a function)
src/init/CMakeFiles/ext_manifest.dir/build.make:62: recipe for target 'src/init/CMakeFiles/ext_manifest.dir/ext_manifest.c.o' failed
make[3]: *** [src/init/CMakeFiles/ext_manifest.dir/ext_manifest.c.o] Error 2
CMakeFiles/Makefile2:2229: recipe for target 'src/init/CMakeFiles/ext_manifest.dir/all' failed

lgirdwood · 2020-09-30T15:42:50Z

@ktrzcinx can you check UT on CI.

This feature will be needed to monitor memory utilization and memory leaks. It may be usable also in realese builds, so removed conditional ipc_glb_test_message compilation. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

This feature is not needed to get functional firmware, so may be disabled for platforms with low memory space, like baytrail and cherrytrail. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

This component is conditionally compiled, so passing such an information to driver, allows to check possible scanning failure reason. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

ktrzcinx · 2020-10-02T10:54:39Z

Reduced number of reserved bytes, as suggested here: thesofproject/linux#2459 (comment)

lgirdwood · 2020-10-03T09:32:26Z

CI is known issue boot issues on ICL. Could be a script race where we may need to cleanup SHM or message Qs.

The command "./scripts/docker-run.sh ./scripts/xtensa-build-all.sh -r $PLATFORM" exited with 0.
4.85s$ ./scripts/docker-qemu.sh ../sof.git/scripts/qemu-check.sh $PLATFORM
./xtensa-softmmu/qemu-system-xtensa -cpu cannonlake -M adsp_cnl -nographic -kernel /home/sof/sof.git/build_cnl_gcc/src/arch/xtensa/sof-cnl.ri -rom /home/sof/sof.git/build_cnl_gcc/src/arch/xtensa/rom-cnl.bin
bridge-io: msg send failed -9
qemu-system-xtensa: terminating on signal 15 from pid 14 (timeout)
ipc reg dump:
00000010  00 00 00 f0 00 00 00 00  80 00 00 00 00 00 00 00  |................|
ipc message dump:
00005000  6c 00 00 00 00 00 00 70  00 00 00 00 00 00 00 00  |l......p........|
00005010  00 00 00 00 00 00 00 00  3c 00 00 00 01 00 05 00  |........<.......|
00005020  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00005030  00 00 00 00 00 00 00 00  00 00 35 37 34 64 32 00  |..........574d2.|
00005040  00 20 01 03 53 08 2f 28  00 00 00 00 00 00 00 00  |. ..S./(........|
Boot success
./xtensa-softmmu/qemu-system-xtensa -cpu icelake -M adsp_icl -nographic -kernel /home/sof/sof.git/build_icl_gcc/src/arch/xtensa/sof-icl.ri -rom /home/sof/sof.git/build_icl_gcc/src/arch/xtensa/rom-icl.bin
qemu-system-xtensa: terminating on signal 15 from pid 40 (timeout)
Error ipc reg failed
Error boot failed

ktrzcinx added the ABI ABI change involved label Sep 4, 2020

ktrzcinx requested review from mmaka1 and zrombel September 4, 2020 06:51

ktrzcinx requested review from a team, bardliao, dbaluta, jajanusz, lbetlej, lgirdwood, libinyang, plbossart, tlauda and xiulipan as code owners September 4, 2020 06:51

kv2019i requested changes Sep 4, 2020

View reviewed changes

src/include/ipc/debug.h Outdated Show resolved Hide resolved

src/include/ipc/header.h Outdated Show resolved Hide resolved

src/include/ipc/debug.h Outdated Show resolved Hide resolved

lyakh reviewed Sep 4, 2020

View reviewed changes

src/ipc/handler.c Outdated Show resolved Hide resolved

ktrzcinx force-pushed the mem-leak branch 4 times, most recently from 7abc2f0 to 6df7310 Compare September 4, 2020 11:14

paulstelian97 reviewed Sep 4, 2020

View reviewed changes

src/ipc/handler.c Outdated Show resolved Hide resolved

ktrzcinx force-pushed the mem-leak branch 2 times, most recently from b53f045 to 655595f Compare September 7, 2020 10:43

ktrzcinx mentioned this pull request Sep 21, 2020

Add memory usage scan possibility thesofproject/linux#2459

Merged

ktrzcinx changed the title ~~[RFC] Add memory usage scan possibility~~ Add memory usage scan possibility Sep 21, 2020

ktrzcinx added 2 commits September 21, 2020 12:40

alloc: Add possibility to read memory usage in runtime

7438741

Such a function may be used to monitor memory leaks and system utilization. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

ipc: Rename function handling SOF_IPC_GLB_TRACE_MSG

178c919

Function name should correspond IPC name. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

ktrzcinx force-pushed the mem-leak branch from 655595f to 3d18f95 Compare September 21, 2020 10:45

lgirdwood added this to the ABI-3.18 milestone Sep 23, 2020

kv2019i approved these changes Sep 25, 2020

View reviewed changes

plbossart approved these changes Sep 25, 2020

View reviewed changes

lgirdwood requested changes Sep 29, 2020

View reviewed changes

ktrzcinx force-pushed the mem-leak branch from 3d18f95 to e5882d6 Compare September 29, 2020 12:35

ktrzcinx force-pushed the mem-leak branch from e5882d6 to 4fe15ed Compare September 29, 2020 14:08

ktrzcinx added 3 commits October 2, 2020 12:53

ipc: debug: Add KConfig to conditionally compile memory scan feature

2e7e5c1

This feature is not needed to get functional firmware, so may be disabled for platforms with low memory space, like baytrail and cherrytrail. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

ext_man: Add information about runtime memory scan possibility

f64bfb1

This component is conditionally compiled, so passing such an information to driver, allows to check possible scanning failure reason. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>

ktrzcinx force-pushed the mem-leak branch from 4fe15ed to f64bfb1 Compare October 2, 2020 10:54

lgirdwood approved these changes Oct 6, 2020

View reviewed changes

lgirdwood merged commit b119fe2 into thesofproject:master Oct 6, 2020

Add memory usage scan possibility #3389

Add memory usage scan possibility #3389

Uh oh!

Conversation

ktrzcinx commented Sep 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marc-hb commented Sep 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgirdwood commented Sep 7, 2020

Uh oh!

lgirdwood commented Sep 25, 2020

Uh oh!

plbossart commented Sep 25, 2020

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

ktrzcinx commented Sep 29, 2020

Uh oh!

lgirdwood commented Sep 29, 2020

Uh oh!

lgirdwood commented Sep 30, 2020

Uh oh!

ktrzcinx commented Oct 2, 2020

Uh oh!

lgirdwood commented Oct 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ktrzcinx commented Sep 4, 2020 •

edited

Loading

marc-hb commented Sep 4, 2020 •

edited

Loading