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

[zhangyanfei]

于 2012年11月07日 05:30, Dave Anderson 写道:
> 
> 
> ----- 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.

Hmm, runq -g includes tasks in throttled cfs_rqs/rt_rqs. Its implementation is
so complex because when dumping a cfs rb_tree, if one of its node is a also child
cfs rb_tree, we follow the child tree and dump all tasks in it. If the child cfs
rb_tree is throttled, it is dequeued from its parent rb_tree and we lost tasks in
this rb_tree. So when dumping the cfs rb_tree, I checked if there is a child cfs
rb_tree throttled, if so, dumping it.
The same thing is for rt_rq.

Dumping group info, such as group name, is to make it clear that which task is
belong to which group.

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

ok. I rewrite the patch and they are tested ok in my box.

Thanks
Zhang
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-add-g-option-for-runq-v4.patch
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20121107/d24b524c/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0002-add-help-info-for-runq-g.patch
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20121107/d24b524c/attachment-0001.ksh>