Skip to content

mm/dump:add command to support dump memory usage of all pids #18468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xiaoqizhan wants to merge 1 commit into
base: master
Choose a base branch
from
+45 −1
Open

mm/dump:add command to support dump memory usage of all pids #18468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion fs/procfs/fs_procfsmeminfo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ static ssize_t memdump_read(FAR struct file *filep, FAR char *buffer,
"/on/off"
#endif
#if CONFIG_MM_BACKTRACE >= 0
"/leak/pid> <seqmin> <seqmax"
"/leak/pid/allpid> <seqmin> <seqmax"
#endif
">\n"
"used: dump all allocated node\n"
Expand All @@ -463,6 +463,7 @@ static ssize_t memdump_read(FAR struct file *filep, FAR char *buffer,
#if CONFIG_MM_BACKTRACE >= 0
"leak: dump all leaked node\n"
"pid: dump pid allocated node\n"
"allpid: dump all pid memory allocated\n"
# ifdef CONFIG_MM_BACKTRACE_SEQNO
"The current sequence number %lu\n",
g_mm_seqno
Expand Down Expand Up @@ -616,6 +617,11 @@ static ssize_t memdump_write(FAR struct file *filep, FAR const char *buffer,
break;

#if CONFIG_MM_BACKTRACE >= 0
case 'a':
Copy link
Contributor

@anchao anchao Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case 'a':
case 'a':

dump.pid = PID_MM_ALL_PID;
p = (FAR char *)buffer + 6;
goto dump;

default:
if (!isdigit(buffer[0]))
{
Expand Down
3 changes: 3 additions & 0 deletions include/malloc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@

/* Special PID to query the info about alloc, free and mempool */

#if CONFIG_MM_BACKTRACE >= 0
Copy link
Contributor

@anchao anchao Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#if CONFIG_MM_BACKTRACE >= 0

remove, definition does not require adding macro check

#define PID_MM_ALL_PID ((pid_t)-7)
#endif
#define PID_MM_ORPHAN ((pid_t)-6)
#define PID_MM_BIGGEST ((pid_t)-5)
#define PID_MM_FREE ((pid_t)-4)
Expand Down
35 changes: 35 additions & 0 deletions mm/mm_heap/mm_memdump.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,29 @@ static void memdump_handler(FAR struct mm_allocnode_s *node, FAR void *arg)
}
}

#if CONFIG_MM_BACKTRACE >= 0
struct memdump_tcb_arg_s
Copy link
Contributor

@wangchdo wangchdo Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put the type definition in section:

/****************************************************************************

  • Private Types
    ****************************************************************************/

Copy link
Contributor

@anchao anchao Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to file begin and add Private Types chapter

{
FAR struct mm_heap_s *heap;
unsigned long seqmin;
unsigned long seqmax;
Comment on lines +230 to +231
Copy link
Contributor

@acassis acassis Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use uintptr_t instead of "unsigned long"? It should be more adaptable to different architectures and data types.

Copy link
Contributor Author

@xiaoqizhan xiaoqizhan Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acassis ,Thank you for your review and feedback. The parameters of the memdump command are passed down via the memdump_write method, with the relevant code in this method as follows:

struct mm_memdump_s dump =
{
PID_MM_ALLOC,
#if CONFIG_MM_BACKTRACE >= 0
0,
ULONG_MAX
#endif
};
The type of mm_memdump_s is struct malltask, whose definition is shown below:

struct malltask
{
pid_t pid; /* Process id /
#if CONFIG_MM_BACKTRACE >= 0
unsigned long seqmin; / The minimum sequence /
unsigned long seqmax; / The maximum sequence */
#endif
};
To maintain consistency with the previous data types, I also use unsigned long for seqmin and seqmax here.

Copy link
Contributor

@acassis acassis Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @xiaoqizhan, so let's keep it for now, but I suggest fixing it later (in another PR) using "uintptr_t" to make it to adapt better for each arch case.

Copy link
Contributor Author

@xiaoqizhan xiaoqizhan Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acassis Thank you for your valuable suggestion. I will try to optimize it in another PR as you recommended.

};

static void memdump_tcb_handler(FAR struct tcb_s *tcb, FAR void *arg)
{
struct mallinfo_task info;
struct malltask task;
FAR struct memdump_tcb_arg_s *tcb_arg = arg;

task.pid = tcb ? tcb->pid : PID_MM_LEAK;
task.seqmin = tcb_arg->seqmin;
task.seqmax = tcb_arg->seqmax;
info = mm_mallinfo_task(tcb_arg->heap, &task);
syslog(LOG_INFO, "pid:%5d, used:%10d, nused:%10d\n",
Copy link
Contributor

@anchao anchao Mar 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
syslog(LOG_INFO, "pid:%5d, used:%10d, nused:%10d\n",
syslog(LOG_INFO, "PID: %5d, Used: %10d, NUsed: %10d\n",

Copy link
Contributor

@anchao anchao Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we filter out PIDs that don't use any memory?

task.pid, info.uordblks, info.aordblks);
}
#endif

/****************************************************************************
* Public Functions
****************************************************************************/
Expand Down Expand Up @@ -301,6 +324,18 @@ void mm_memdump(FAR struct mm_heap_s *heap,
{
syslog(LOG_INFO, "Dump allocated orphan nodes\n");
}
#if CONFIG_MM_BACKTRACE >= 0
else if (pid == PID_MM_ALL_PID)
{
syslog(LOG_INFO, "Dump all pid memory allocated\n");
struct memdump_tcb_arg_s tcb_arg;
tcb_arg.heap = heap;
tcb_arg.seqmin = dump->seqmin;
tcb_arg.seqmax = dump->seqmax;
nxsched_foreach(memdump_tcb_handler, &tcb_arg);
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not reuse memdump_handler, the current approach has two loop(tcb + mem) which is very slower than one loop.

Copy link
Contributor Author

@xiaoqizhan xiaoqizhan Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xiaoxiang781216
Thank you for your review and feedback. If the memdump_handler method is used, it will further call memdump_allocnode, which dumps detailed information of each node (including call stacks and other logs), which does not meet the current requirements. Additionally, in my patch, it calls mm_foreach(heap, mallinfo_task_handler, &handle) to traverse all memory nodes of the heap and perform statistics, without recording or outputting any other extra information.
In terms of the number of loops: a total of 3 loops are required: nxsched_foreach traverses the corresponding pid, mm_foreach traverses mm_nregions of the corresponding heap, and then traverses memory nodes based on each region to execute the callback function for statistics. The implementations of memdump_handler and mallinfo_task_handler are basically similar.
In the native implementation, when using the memdump pid command, traversal is performed via mm_foreach(heap, memdump_handler, &priv), which also involves two loops: traversing mm_nregions of the corresponding heap, and then traversing memory nodes based on each region. To obtain the memory information for all PIDs, an additional loop level is required, so the total number of loops remains three.

Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, but why not pass dump to nxsched_foreach and remove memdump_tcb_arg_s.
BTW, it's better to integrate this feature to meminfo_read with an Kconfig(e.g. CONFIG_FS_PROCFS_MEMINFO_DETAIL) by extending procfs_meminfo_entry_s::mallinfo to procfs_meminfo_entry_s::mallinfo_task

return;
}
#endif

#if CONFIG_MM_BACKTRACE < 0
syslog(LOG_INFO, "%12s%9s%*s\n", "Size", "Overhead",
Expand Down
Loading