Re: [PATCH 09/18] io-wq: fork worker threads from original task

[Jens Axboe <axboe@kernel.dk>]

On 3/4/21 5:23 AM, Stefan Metzmacher wrote:
> 
> Hi Jens,
> 
>> +static pid_t fork_thread(int (*fn)(void *), void *arg)
>> +{
>> +	unsigned long flags = CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|
>> +				CLONE_IO|SIGCHLD;
>> +	struct kernel_clone_args args = {
>> +		.flags		= ((lower_32_bits(flags) | CLONE_VM |
>> +				    CLONE_UNTRACED) & ~CSIGNAL),
>> +		.exit_signal	= (lower_32_bits(flags) & CSIGNAL),
>> +		.stack		= (unsigned long)fn,
>> +		.stack_size	= (unsigned long)arg,
>> +	};
>> +
>> +	return kernel_clone(&args);
>> +}
> 
> Can you please explain why CLONE_SIGHAND is used here?

We can't have CLONE_THREAD without CLONE_SIGHAND... The io-wq workers
don't really care about signals, we don't use them internally.

> Will the userspace signal handlers executed from the kernel thread?

No

> Will SIGCHLD be posted to the userspace signal handlers in a userspace
> process? Will wait() from userspace see the exit of a thread?

Currently actually it does, but I think that's just an oversight. As far
as I can tell, we want to add something like the below. Untested... I'll
give this a spin in a bit.

diff --git a/kernel/signal.c b/kernel/signal.c
index ba4d1ef39a9e..e5db1d8f18e5 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1912,6 +1912,10 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
 	bool autoreap = false;
 	u64 utime, stime;
 
+	/* Don't notify a parent task if an io_uring worker exits */
+	if (tsk->flags & PF_IO_WORKER)
+		return true;
+
 	BUG_ON(sig == -1);
 
  	/* do_notify_parent_cldstop should have been called instead.  */

-- 
Jens Axboe