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

Dave Anderson anderson at redhat.com
Tue Nov 6 21:30:18 UTC 2012



----- Original Message -----

> >  
> >>
> >> 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.
> 
> 
> Two patches added. One is the fixed version for v2 and the other is
> adding the help info for run -g.
> 
> Thanks
> Zhang

These latest patches tested OK.

However, I must admit that I don't really understand the details w/respect
to the difference between the "runq" and "runq -g" output other than the 
two different options essentially find all per-cpu queued tasks coming from
different rb_root structures.  Also, the "runq -g" implementation is remarkably
complex in comparison to the "runq" alone, making its future maintenance a 
potentially difficult chore because of the huge number of structure.member
dependencies.

Now, for the most part, you have been able to segregate the code, *except*
for the overloading of the currently-existing dump_tasks_in_cfs_rq() function to
handle both the "runq" and the "runq -g" commands, i.e., where you've merged in 
the g_flag and depth business.  And as a result, it makes the patched "dual-mode"
dump_tasks_in_cfs_rq() function difficult to understand.

Here is my suggestion/request:

(1) please leave dump_CFS_runqueues() and dump_tasks_in_cfs_rq() as they are now.
(2) create a new (somewhat redundant) "dump_tasks_in_task_group_rq()" function that is only
    used for "runq -g".
(3) have your new dump_tasks_by_task_group() function call the new dump_tasks_in_task_group_rq()
    function.

That would completely segregate the implementations of the two options.  The worst
case for doing that it that way is that there will be dump_tasks_in_cfs_rq() and 
and dump_tasks_in_task_group_rq() functions that are somewhat similar.  But considering
the huge amount of code that you are adding for "runq -g", having separate functions is
hardly a cause for concern.  And it should be fairly trivial for you to update your patch
to do it that way.
 
And then you could effectively do the same type of code separation for the RT queues.

Does that make sense to you?

Dave





More information about the Crash-utility mailing list