Repository navigation
Conversation
Add vmh_log_alloc_stats() which prints only a single-line summary of total allocated/free bytes; call it on every successful allocation in virtual_heap_alloc() instead of the verbose vmh_log_stats(). Both functions are compiled only when CONFIG_SYS_MEM_BLOCKS_RUNTIME_STATS is set. This matches the debug capability in place for plain heap allocs. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Invalid vregion states can query uninitialized heap metadata, and the overlay does not reliably enable its INFO-level logs.
Review effort: Balanced
Findings: 3
Open (4)
What changed in this PR
Adds allocation-time diagnostics across SOF heap implementations and a unified debugging overlay.
Changes:
- Logs vregion allocation statistics.
- Adds aggregate virtual-heap usage logging after allocations.
- Provides an overlay enabling runtime heap statistics.
| File | Description |
|---|---|
zephyr/lib/vregion.c |
Logs lifetime and interim heap usage. |
zephyr/lib/regions_mm.c |
Aggregates virtual-heap allocator statistics. |
zephyr/lib/alloc.c |
Logs statistics after virtual-heap allocations. |
zephyr/include/sof/lib/regions_mm.h |
Declares the statistics helper. |
app/debug_heap_allocs.conf |
Enables runtime statistics options. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| CONFIG_SYS_HEAP_RUNTIME_STATS=y | ||
| CONFIG_SYS_MEM_BLOCKS_RUNTIME_STATS=y |
There was a problem hiding this comment.
Adjusted the comment to indicate what level of logs are emitted. We do not want to enable logs in this overlay.
| #if CONFIG_SYS_HEAP_RUNTIME_STATS | ||
| if (vr->type == VREGION_MEM_TYPE_LIFETIME) { | ||
| LOG_INF("lifetime alloc of %zu, used %zu, free %zu", | ||
| size, vr->lifetime.used, | ||
| vr->lifetime.size - vr->lifetime.used); |
There was a problem hiding this comment.
This is ok for a debug feature like this.
| } else { | ||
| struct sys_memory_stats stats; | ||
|
|
||
| sys_heap_runtime_stats_get(&vr->interim.heap.heap, &stats); |
| heap->logged = true; | ||
| } | ||
|
|
||
| void vmh_log_alloc_stats(struct vmh_heap *heap) |
There was a problem hiding this comment.
Skipping this for internal helper.
PR 11264: test resultsRun date: 2026-10-08 12:27 UTC Tested commit: 3dec250263b83c3350bbb6ff39277e1da0d25764 |
| } else { | ||
| struct sys_memory_stats stats; | ||
|
|
||
| sys_heap_runtime_stats_get(&vr->interim.heap.heap, &stats); | ||
|
|
||
| LOG_INF("interim alloc of %zu, used %u, free %u, max %u", | ||
| size, stats.allocated_bytes, stats.free_bytes, stats.max_allocated_bytes); | ||
|
|
||
| } |
There was a problem hiding this comment.
Is it possible to get here with vr->type == VREGION_MEM_TYPE_INVALID? switch in line 503 suggests so. In that case these structures are probably not correctly populated.
There was a problem hiding this comment.
Ack, fixed this in V2.
| struct sys_memory_stats stats; | ||
|
|
||
| sys_heap_runtime_stats_get(&vr->interim.heap.heap, &stats); | ||
|
|
||
| LOG_INF("interim alloc of %zu, used %u, free %u, max %u", | ||
| size, stats.allocated_bytes, stats.free_bytes, stats.max_allocated_bytes); |
There was a problem hiding this comment.
sys_heap_runtime_stats_get can return -EINVAL. You can initialize stats to avoid printing garbage in the log.
There was a problem hiding this comment.
Likewise, fixed in V2, thanks.
|
One complaint I got regarding my solution was that the logs are never displayed... because my intention was to provide a function that could be used ad hoc during debugging (e.g. called while deleting a pipeline). On the other hand, your solution displays the stats unconditionally. But if you confirmed it doesn't overwhelm the logging interface, I guess that's fine... EDIT: |
Add a single-line LOG_INF to z_impl_vregion_alloc_align() reporting the lifetime allocator's used and free bytes after each successful allocation. Gated on CONFIG_SYS_HEAP_RUNTIME_STATS. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add a debug overlay file to enable alloc heap debugging, with prints of memory usage via logging subsystem. This is added as a separate file as this creates a notable increase in logging traffic and is not something one wants enabled in all builds (e.g. depends on the logging backend bandwidth). Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
b5363b4 to
3dec250
Compare
|
@wjablon1 wrote:
Ack. I have the same problem here. I chose not to put this into debug overlay (which is used in some validation configurations), but instead a separate overlay (= used by developers for specific tasks). The amount of logs generated is enough to flood the mtrace window (on Intel DSPs) for sure, so unless you switch the logging backend (e.g. use a mipi syst dictionary logging backend) you can miss important logs. So definitely not something that can be left enabled by default.
Ack. I think for the per allocator stats, your existing print-on-error already covers this. I found the total interesting when sizing up a new configuration (adding/removing modules, moving stuff in/out of DRAM) and running tests to see how much runtime heap different use-cases need (and then iterate on the config until you find a working configuration with enough features built in to the FW, and still enough heap space). |


A series to help debugging heap usage issues. We already have infra to print logs for each alloc, but these were not documented for developers and didn't cover all the heaps (especially not newer "virtual heap" and "vregion").
This series add similar debugging for all major heap implementations (print out usage stats at each alloc), and adds an overlay to enable these all with a single overlay file.