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

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