Add Ps crash command #519

YassineLr · 2025-07-10T11:48:11Z

No description provided.

YassineLr · 2025-07-10T15:21:05Z

Hey, I added some helper functions but couldn’t find kernel equivalents for example they usually access virtual memory size directly from task_struct rather than wrapping it in a helper.
I haven’t added any tests yet -- waiting to see if you think these helpers are worth keeping. Thank you !

osandov

I only looked at the helpers this time around, a few comments there. I know you're just adapting these from drgn-tools, so thanks for your patience!

drgn/helpers/linux/pid.py

drgn/helpers/linux/mm.py

brenns10

Sorry @YassineLr and @osandov , I meant to get this out sooner. I left out comments on the helpers since Omar took a look there. I did some light review on the ps implementation itself.

drgn/commands/_builtin/crash/ps.py

YassineLr · 2025-07-15T12:16:41Z

Thanks a lot @osandov and @brenns10 for the detailed comments! I’ll go through each change you mentioned.

YassineLr · 2025-08-08T14:29:30Z

Hey @osandov @brenns10 Apologies for the delay. I’ve addressed the requested changes, please let me know if there’s anything else you’d like me to adjust.

osandov

Thanks, this is a cool command. I left some comments throughout, some about the output format, some about the helpers, some on the code style.

The commits also need to be cleaned up. Please rebase to get rid of those merge commits and split the helpers out into their own commit (you can have a separate commit per helper or just put them together, up to you, but please have them separate from the actual command). The helpers also need test cases.

drgn/commands/_builtin/crash/ps.py

osandov · 2025-09-02T17:26:51Z

drgn/helpers/linux/mm.py

    return TaskRss(filerss, anonrss, shmemrss, swapents)
+
+
+def task_vsize(task: Object) -> float:


Doesn't this return an int?

drgn/helpers/linux/kthread.py

osandov · 2025-09-02T17:29:23Z

drgn/helpers/linux/pid.py

            yield other
+
+
+def is_group_leader(t: Object) -> bool:


The kernel function is named thread_group_leader(), so let's use that name. The kernel function is in include/linux/sched/signal.h, so let's move this to drgn.helpers.linux.sched, too.

osandov · 2025-09-02T17:29:38Z

drgn/helpers/linux/pid.py

+
+def is_group_leader(t: Object) -> bool:
+    """
+    Check if a task is thread group leader.


Document argument type:

Suggested change

Check if a task is thread group leader.

Check if a task is thread group leader.

:param task: ``struct task_struct *``

YassineLr · 2025-09-29T17:25:20Z

Hey @osandov @brenns10 , sorry for the delay. I’ve addressed the requested changes and looking forward to your review.

osandov

Thanks, Yassine, sorry for my slow turnaround!

drgn/helpers/linux/mm.py

drgn/helpers/linux/sched.py

tests/linux_kernel/helpers/test_kthread.py

drgn/helpers/linux/mm.py

osandov · 2025-11-08T00:09:33Z

drgn/commands/_builtin/crash/ps.py

+                "-u",
+                dest="user",
+                action="store_true",
+                default=False,


default is automatically False when action="store_true", so you can drop this (and all of the other ones).

Suggested change

default=False,

osandov · 2025-11-08T00:28:57Z

drgn/commands/_builtin/crash/ps.py

+        if args.user or args.kernel:
+            builder.add_from_import("drgn.helpers.linux.kthread", "is_kthread")
+            if args.user:
+                builder.append(
+                    "tasks = filter(lambda t: not task_is_kthread(t), for_each_task(prog))\n"
+                )
+            elif args.kernel:
+                builder.append("tasks = filter(task_is_kthread, for_each_task(prog))\n")
+        else:
+            builder.append("tasks = for_each_task(prog)\n")
+
+        if args.group_leader:
+            builder.add_from_import(
+                "drgn.helpers.linux.sched",
+                "thread_group_leader",
+            )
+            builder.append("tasks = filter(thread_group_leader, tasks)\n")


Rather than applying these as filters, I think it'd look nicer to make them checks in the for loop. E.g., something like

for task in for_each_task(): if task_is_kthread(task): continue if not thread_group_leader(task): continue

It also needs to consider the pid/task/command checks.

osandov · 2025-11-08T00:29:38Z

drgn/commands/_builtin/crash/ps.py

+        builder.append("    total_rss = task_rss.total\n")
+        builder.append("    pct_mem = total_rss * 100 / total_mem\n")
+        builder.append("    vmem = task_vsize(task)\n")
+        builder.append("    command = escape_ascii_string(task.comm.string_())\n")


No need to convert this to a string here, just

Suggested change

builder.append(" command = escape_ascii_string(task.comm.string_())\n")

builder.append(" command = task.comm\n")

(And you can remove the add_from_import for escape_ascii_string).

osandov · 2025-11-08T00:30:14Z

drgn/commands/_builtin/crash/ps.py

+            builder.append(
+                "    last_arrival = format_nanosecond_duration(time_nanosec)\n"
+            )


format_nanosecond_duration() isn't a public helper. We don't really need to format this, either, so you can just delete it

Suggested change

builder.append(

" last_arrival = format_nanosecond_duration(time_nanosec)\n"

)

osandov · 2025-11-08T00:30:51Z

drgn/commands/_builtin/crash/ps.py

+            builder.add_from_import(
+                "drgn.helpers.linux.sched", "task_since_last_arrival_ns"
+            )
+            builder.add_import("datetime")


This isn't used anywhere, did you intend to use it?

osandov · 2025-11-08T00:31:21Z

drgn/commands/_builtin/crash/ps.py

+    print_table,
+)
+from drgn.helpers.linux.kthread import task_is_kthread
+from drgn.helpers.linux.mm import get_task_rss_info, task_vsize, totalram_pages


FYI I renamed get_task_rss_info() to task_rss_info(), so you'll want to rebase and update that.

…task() It's often surprising that for_each_task() doesn't actually return every task_struct, as it omits PID 0. It's probably too late now to change the default to include idle threads, but make it an option. The crash ps command will also use this. Signed-off-by: Omar Sandoval <[email protected]>

… and environ() For the crash ps command, it'll be slightly more convenient to get a task's mm and explicitly test it rather than checking for a None return. Signed-off-by: Omar Sandoval <[email protected]>

It's awkward to build the loop body to pass to DrgnCodeBuilder.{append,wrap}_retry_loop_if_live() and CrashDrgnCodeBuilder.append_cpuspec(). Replace them with context-based APIs that automatically add indentation to subsequent appends as needed. Signed-off-by: Omar Sandoval <[email protected]>

This is needed for the crash ps and foreach commands. Unfortunately, we're forced to hard-code the PF_KTHREAD flag. Signed-off-by: YassineLr <[email protected]> [Omar: rewrite test case] Signed-off-by: Omar Sandoval <[email protected]>

This is needed for the crash ps command. Signed-off-by: YassineLr <[email protected]>

This is needed for the crash ps and foreach commands. Signed-off-by: YassineLr <[email protected]> [Omar: rewrite test case] Signed-off-by: Omar Sandoval <[email protected]>

I believe that this is complete. The implementation of -s is terrible; it does a full stack trace just to get the initial stack pointer, but we can optimize that later. Co-authored-by: YassineLr <[email protected]> Signed-off-by: YassineLr <[email protected]> Signed-off-by: Omar Sandoval <[email protected]>

osandov requested changes Jul 14, 2025

View reviewed changes

brenns10 suggested changes Jul 15, 2025

View reviewed changes

brenns10 mentioned this pull request Jul 15, 2025

Migrate runq related helpers from drgn-tools #522

Merged

YassineLr force-pushed the dev/ps branch from 71eb640 to 739fe89 Compare August 8, 2025 16:46

YassineLr requested a review from osandov August 19, 2025 19:02

osandov requested changes Sep 2, 2025

View reviewed changes

YassineLr force-pushed the dev/ps branch 2 times, most recently from e206a01 to d648e61 Compare September 29, 2025 17:20

osandov requested changes Nov 8, 2025

View reviewed changes

osandov linked an issue Dec 8, 2025 that may be closed by this pull request

Crash command: ps #509

Closed

osandov and others added 7 commits December 11, 2025 11:14

drgn.helpers.linux.mm: factor mm_{cmdline,environ}() out of cmdline()…

8962fc8

… and environ() For the crash ps command, it'll be slightly more convenient to get a task's mm and explicitly test it rather than checking for a None return. Signed-off-by: Omar Sandoval <[email protected]>

drgn.helpers.linux.kthread: add task_is_kthread()

ddabfc4

This is needed for the crash ps and foreach commands. Unfortunately, we're forced to hard-code the PF_KTHREAD flag. Signed-off-by: YassineLr <[email protected]> [Omar: rewrite test case] Signed-off-by: Omar Sandoval <[email protected]>

drgn.helpers.linux.mm: add task_vsize()

601272c

This is needed for the crash ps command. Signed-off-by: YassineLr <[email protected]>

drgn.helpers.linux.sched: add thread_group_leader()

b0b3f89

This is needed for the crash ps and foreach commands. Signed-off-by: YassineLr <[email protected]> [Omar: rewrite test case] Signed-off-by: Omar Sandoval <[email protected]>

osandov force-pushed the dev/ps branch from d648e61 to af38c76 Compare December 11, 2025 22:47

osandov merged commit dc391e5 into osandov:main Dec 11, 2025
35 checks passed

		return TaskRss(filerss, anonrss, shmemrss, swapents)


		def task_vsize(task: Object) -> float:

	builder.append(" command = escape_ascii_string(task.comm.string_())\n")
	builder.append(" command = task.comm\n")

	builder.append(
	" last_arrival = format_nanosecond_duration(time_nanosec)\n"
	)

Add Ps crash command #519

Add Ps crash command #519

Conversation

YassineLr commented Jul 10, 2025

Uh oh!

YassineLr commented Jul 10, 2025

Uh oh!

osandov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brenns10 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

YassineLr commented Jul 15, 2025

Uh oh!

YassineLr commented Aug 8, 2025

Uh oh!

osandov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YassineLr commented Sep 29, 2025

Uh oh!

osandov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants