Messages in this thread | | | From | (Eric W. Biederman) | Subject | Re: [PATCH 1/2] kill_something_info: misc cleanups | Date | Sun, 17 Dec 2006 04:22:48 -0700 |
| |
Christoph Hellwig <hch@infradead.org> writes:
> On Sat, Dec 16, 2006 at 11:05:10PM +0300, Oleg Nesterov wrote: >> static int kill_something_info(int sig, struct siginfo *info, int pid) >> { >> int ret; >> + >> + rcu_read_lock(); >> + if (pid > 0) { >> + ret = kill_pid_info(sig, info, find_pid(pid)); >> + } else if (pid == -1) { >> + struct task_struct *p; >> + int found = 0; >> + >> + ret = 0; >> + read_lock(&tasklist_lock); >> + for_each_process(p) >> + if (!is_init(p) && p != current->group_leader) { >> + int err = group_send_sig_info(sig, info, p); >> + if (err != -EPERM) >> + ret = err; >> + found = 1; >> + } >> + read_unlock(&tasklist_lock); >> + if (!found) >> + ret = -ESRCH; > > This branch should probably be factored out into a helper of it's own:
The proper name would be something like kill_all_info(). As we are talking about the group of all processes.
I am sitting here wondering why we bother to ignore init, as init is protected from all signals it doesn't explicitly setup a signal handler for. It is probably worth taking a quick look at the common shutdown scripts and sysv init and see if anything actually cares if we simply remove the is_init check.
The only two signals I know that are commonly handled this way are kill(-1, SIGTERM) and kill(-1, SIGKILL);
And a very quick look at sysvinit-2.86 shows that it doesn't setup a handler for SIGTERM. So I believe we can delete we can delete the is_init check entirely without changing anything and with a less surprising if anyone ever cares.
>> + } else { >> + struct pid *grp = task_pgrp(current); >> + if (pid != 0) >> + grp = find_pid(-pid); >> + ret = kill_pgrp_info(sig, info, grp); >> + } > > This also looks rather unreadable, an > > } else if (pid) { > ret = kill_pgrp_info(sig, info, find_pid(-pid)); > } else { > ret = kill_pgrp_info(sig, info, task_pgrp(current)); > } > > might be slightly more code, but also a lot more readable.
And that part is basically what we have now, just reshuffled.
Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |