Re: [Crash-utility] [PATCH] runq: make tasks in throttled cfs_rqs/rt_rqs displayed
----- Original Message -----
>>
>> I worry about the large number of kernel structure.member declarations that the
>> command depends upon, because if just one of them changes, it breaks the
command.
>> So it would be preferable if the "runq" command (with no arguments)
would only use
>> a minimal set of structure.member offsets.
I made two new offset-init functions for runq:
static void cfs_rq_offset_init(void);
static void task_group_offset_init(void);
One minor suggestion -- it's really not necessary to call MEMBER_EXISTS()
prior to calling MEMBER_OFFSET_INIT():
diff --git a/kernel.c b/kernel.c
index 45da48e..868d777 100755
--- a/kernel.c
+++ b/kernel.c
@@ -308,6 +308,12 @@ kernel_init()
STRUCT_SIZE_INIT(prio_array, "prio_array");
MEMBER_OFFSET_INIT(rq_cfs, "rq", "cfs");
+ if (MEMBER_EXISTS("task_group", "cfs_rq"))
+ MEMBER_OFFSET_INIT(task_group_cfs_rq, "task_group",
"cfs_rq");
+ if (MEMBER_EXISTS("task_group", "rt_rq"))
+ MEMBER_OFFSET_INIT(task_group_rt_rq, "task_group",
"rt_rq");
+ if (MEMBER_EXISTS("task_group", "parent"))
+ MEMBER_OFFSET_INIT(task_group_parent, "task_group",
"parent");
You can simply just call MEMBER_OFFSET_INIT() the 3 times above. If the structure
members don't exist, MEMBER_OFFSET_INIT() will just set the offsets to -1
(INVALID_OFFSET).
The same thing applies in your task_group_offset_init() function:
+ if (MEMBER_EXISTS("task_group", "cfs_bandwidth")) {
+ MEMBER_OFFSET_INIT(task_group_cfs_bandwidth,
+ "task_group", "cfs_bandwidth");
+ MEMBER_OFFSET_INIT(cfs_rq_throttled, "cfs_rq",
+ "throttled");
+ }
+
+ if (MEMBER_EXISTS("task_group", "rt_bandwidth")) {
+ MEMBER_OFFSET_INIT(task_group_rt_bandwidth,
+ "task_group", "rt_bandwidth");
+ MEMBER_OFFSET_INIT(rt_rq_rt_throttled, "rt_rq",
+ "rt_throttled");
+ }
Sometimes it helps to call MEMBER_EXISTS() for clarity's sake, but it's
actually kind of redundant.
>
> And to follow up, I'm still running tests (and will do so overnight) on your
latest
> patch, but I immediately see this on any 2.6.30, 2.6.31 or 2.6.32 kernel, and on
> some 2.6.36 and 2.6.38 kernels, where "runq -g" fails like this:
>
> crash> runq -g
> runq: invalid kernel virtual address: 0 type: "dentry"
> crash>
>
As you have pointed in another mail, this is fixed in patch v2, I think.
Yes, that's fixed this time...
> And we certainly want to keep the group information separate from
> the normal "runq" command. Here on a "live" 4 cpu Fedora 3.6.3
kernel,
> the command output exceeds 1000 lines! I'm pretty sure that most
> people will *not* want to see all of this:
>
> crash> runq -g
> CPU 0
> CURRENT: PID: 0 TASK: ffffffff81c13420 COMMAND: "swapper/0"
> RT PRIO_ARRAY: ffff88021e213e28
> [100] GROUP RT PRIO_ARRAY: ffff88020db22000 <system>
> [100] GROUP RT PRIO_ARRAY: ffff8801f8180000 <udisks2.service>
> [no tasks queued]
<snip>
> [no tasks queued]
> GROUP CFS RB_ROOT: ffff88020a844128
> [no tasks queued]
> crash>
>
> In fact, it's difficult to actually find a real *task* that is on
> a run queue from among all of the empty "[no tasks queued]" groups!
>
I've noticed this in the first patch and patch V2 has fixed this.
OK, although I didn't realize that was a bug? What were all of those
empty groups that are no longer shown? Are they actually *not* queued?
So I attach the new patch v2 version for runq -g. If you still find
any bug in your tests or have any suggestion about it, that's very
helpful.
TODO:
1. The help info about the -g option.
2. Change rt_rq tasks displayed non-hierarchically.
Like I mentioned above, the latest patch does not change the default
behavior of runq alone, and "runq -g" is not as verbose as the last
patch, which I presume is your intent.
Thanks,
Dave