[Crash-utility] [PATCH] runq: make tasks in throttled cfs_rqs/rt_rqs displayed

[Dave Anderson]

----- 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