Hi Fabio and all, Thus wrote Fabio M. De Francesco (fmdefrancesco@xxxxxxxxx): > On Sunday, October 17, 2021 12:29:02 PM CEST Phillip Potter wrote: > > So I myself am a little confused on this one :-) > > Based on my understanding, so correct me if I'm wrong, a process > > (kthread or otherwise) can still be killed if marked TASK_KILLABLE, > > even if ignoring SIGTERM. Indeed, from a userspace perspective, > > SIGKILL is unblockable anyway - although of course kernel code can > > choose how to respond to it. > Correct. And it seems that by default, a kthread can't be killed with SIGKILL. > > So in other words, the kthread could still be killed while waiting > > in the wait_for_completion_killable() call, even if we are ignoring > > SIGTERM. From that perspective I guess, it is therefore not 'incorrect' as > > such - if indeed we wanted that behaviour. > No. This misunderstandings is my fault. :( > In Martin's patch I read "SIGTERM" but for some reason I thought he was > talking of "SIGKILL". > At the moment, without Martin's patch, the kthread can be terminated by the > command "kill -TERM <PID>". If we try "kill -KILL <PID>", nothing happens. > This is because only "allow_signal(SIGTERM);" is present in the code. Exactly. And this is probably not by intention. It would be consistent to either allow both or none - the latter makes more sense, and it's what most other drivers do. > I think that kthreads must also allow SIGKILL with "allow_signal(SIGKILL);" > for allowing root to make them terminate. Probably. nfsd seems to do this. > For what relates to my patch, it doesn't matter if I either leave > wait_for_completion_killable() as-is or change it to wait_for_completion(). > This is because at the moment SIGKILL cannot kill rtw_cmd_thread(), while > SIGTERM can. > However, for consistency, I should better change it to the uninterruptible > version. That makes sense to me. Let's see what Greg and others say... Best regards, Martin
