Re: [Crash-utility] New option -g with sig

Olivier Daudel wrote: Hi Olivier, As far as the command output, for the sake of consistency, the only thing I'd like to see added would be the task header of the group leader as the first line of the output, and it seems to me that all the pre-thread output should be indented. So, taking your example, it would look like: crash> sig -g 27491 PID: 27489 TASK: f606b560 CPU: 0 COMMAND: "sig_procthread" SIGNAL_STRUCT: f78f8380 COUNT: 3 SIG SIGACTION HANDLER MASK FLAGS [1] c1f3f104 SIG_DFL 0000000000000000 0 ... PID: 27489 TASK: f606b560 CPU: 0 COMMAND: "sig_procthread" SIGPENDING: no ... PID: 27490 TASK: f7d09020 CPU: 0 COMMAND: "sig_procthread" SIGPENDING: no ... PID: 27491 TASK: f544f020 CPU: 1 COMMAND: "sig_procthread" SIGPENDING: no ... crash> However, as far as your implementation goes, I'd prefer not to mix the "ps" and "sig" functionality in the singular show_tgid_list() function. There are a few reasons: (1) it makes the function hard to understand and maintain. (2) the "ps" command is not context-sensitive, while the "sig" command is. (3) the "sig" command is accessible from "foreach sig", but the foreach() command currently doesn't pass any flags into the "sig" handling functions. And speaking of "foreach", "foreach sig -g" would be an ideal way to display signal handling in a per-thread-group manner. However, support for "foreach sig -g" would be a little tricky, because as it is now, foreach() would pass *all* tasks to the signal handling function -- which would redundantly show the same information for each task in a multi-threaded process, and we don't want that... So there would have to be a mechanism where a list of tgids from among the larger list of tasks would have to gathered first, and then only that tgid list would be passed on to the signal displaying code. Currently the foreach() function does this in a loop for each task that is selected: case FOREACH_SIG: pc->curcmd = "sig"; do_sig(tc->task, FOREACH_SIG, fd->reference ? ref : NULL); break; If the -g flag is supported, it could do something like this instead, and post-process the subset of tgid's taken from the full task list: case FOREACH_SIG: if (fd->flags & FOREACH_g_FLAG) { tgid = task_tgid(tc->task); /* * Add tgid some list of tgid's, * ignoring tgid if it's already * on the list... */ add_tgid_to_list(tgid, tgid_list); } else { pc->curcmd = "sig"; do_sig(tc->task, FOREACH_SIG, fd->reference ? ref : NULL); } break; The memory buffer for the list of tgid's could be pre-allocated in the pre-processing part of foreach(), perhaps by GETBUF()'ing enough space for one-tgid-per-the-total-number-of-tasks running on the system. RUNNING_TASKS() will give you that number, and that would guarantee that the buffer would be large enough. Then, in the post-processing part of foreach(), it could call your new signal-handling function for each tgid in the list. In any case, regardless whether you want to implement the -g support from "foreach sig", please update your patch to not modify show_tgid_list(), but instead create a new, similar, signal-specific function that you can call from cmd_sig(). I can write the "foreach" part if you don't want to bother with that... And go ahead and tinker with the "sig" help pages as well... Thanks, Dave