Oleg Nesterov <oleg@xxxxxxxxxx> writes: > On 06/13, Eric W. Biederman wrote: >> >> Oleg Nesterov <oleg@xxxxxxxxxx> writes: >> >> > The comment above the idr_for_each_entry_continue() loop tries to explain >> > why we have to signal each thread in the namespace, but it is outdated. >> > This code no longer uses kill_proc_info(), we have a target task so we can >> > check thread_group_leader() and avoid the unnecessary group_send_sig_info. >> > Better yet, we can change pid_task() to use PIDTYPE_TGID rather than _PID, >> > this way it returns NULL if this pid is not a group-leader pid. >> > >> > Also, change this code to check SIGNAL_GROUP_EXIT, the exiting process / >> > thread doesn't necessarily has a pending SIGKILL. Either way these checks >> > are racy without siglock, so the patch uses data_race() to shut up KCSAN. >> >> You remove the comment but the meat of what it was trying to say remains >> true. For processes in a session or processes is a process group a list >> of all such processes is kept. No such list is kept for a pid >> namespace. So the best we can do is walk through the allocated pid >> numbers in the pid namespace. > > OK, I'll recheck tomorrow. Yet I think it doesn't make sense to send > SIGKILL to sub-threads, and the comment looks misleading today. This was > the main motivation, but again, I'll recheck. Yes, we only need to send SIGKILL to only one thread. Of course there are a few weird cases with zombie leader threads, but I think the pattern you are using handles them. >> It would also help if this explains that in the case of SIGKILL >> complete_signal always sets SIGNAL_GROUP_EXIT which makes that a good >> check to use to see if the process has been killed (with SIGKILL). > > Well, if SIGNAL_GROUP_EXIT is set we do not care if this process was > killed or not. It (the whole thread group) is going to exit, that is all. > > We can even remove this check, it is just the optimization, just like > the current fatal_signal_pending() check. I just meant that the optimization is effective because group_send_sig_info calls complete_signal which sets SIGNAL_GROUP_EXIT. Which makes it an almost 100% accurate test, which makes it a very good optimization. Especially in the case of multi-threaded processes where the code will arrive there for every thread. Eric
