[PATCH v11 4/8] fork: Add USER_WORKER flag to ignore signals

Mike Christie posted 8 patches 2 years, 7 months ago
[PATCH v11 4/8] fork: Add USER_WORKER flag to ignore signals
Posted by Mike Christie 2 years, 7 months ago
From: Christian Brauner <brauner@kernel.org>

Since:

commit 10ab825bdef8 ("change kernel threads to ignore signals instead of
blocking them")

kthreads have been ignoring signals by default, and the vhost layer has
never had a need to change that. This patch adds an option flag,
USER_WORKER_SIG_IGN, handled in copy_process() after copy_sighand()
and copy_signals() so vhost_tasks added in the next patches can continue
to ignore singals.

Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/linux/sched/task.h | 1 +
 kernel/fork.c              | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 18e614591c24..ce6240a006cf 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -21,6 +21,7 @@ struct css_set;
 #define USER_WORKER		BIT(0)
 #define USER_WORKER_IO		BIT(1)
 #define USER_WORKER_NO_FILES	BIT(2)
+#define USER_WORKER_SIG_IGN	BIT(3)
 
 struct kernel_clone_args {
 	u64 flags;
diff --git a/kernel/fork.c b/kernel/fork.c
index bb98b48bc35c..55c77de45271 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2287,6 +2287,9 @@ static __latent_entropy struct task_struct *copy_process(
 	if (retval)
 		goto bad_fork_cleanup_io;
 
+	if (args->worker_flags & USER_WORKER_SIG_IGN)
+		ignore_signals(p);
+
 	stackleak_task_init(p);
 
 	if (pid != &init_struct_pid) {
-- 
2.25.1
Re: [PATCH v11 4/8] fork: Add USER_WORKER flag to ignore signals
Posted by Linus Torvalds 2 years, 7 months ago
On Thu, Feb 2, 2023 at 3:25 PM Mike Christie
<michael.christie@oracle.com> wrote:
>
> +       if (args->worker_flags & USER_WORKER_SIG_IGN)
> +               ignore_signals(p);

Same comment as for the other case.

There are real reasons to avoid bitfields:

 - you can't pass addresses to them around

 - it's easier to read or assign multiple fields in one go

 - they are horrible for ABI issues due to the exact bit ordering and
padding being very subtle

but none of those issues are relevant here, where it's a kernel-internal ABI.

All these use-cases seem to actually be testing one bit at a time, and
the "assignments" are structure initializers for which named bitfields
are actually perfect and just make the initializer more legible.

            Linus
Re: [PATCH v11 4/8] fork: Add USER_WORKER flag to ignore signals
Posted by Mike Christie 2 years, 7 months ago
On 2/2/23 6:19 PM, Linus Torvalds wrote:
> On Thu, Feb 2, 2023 at 3:25 PM Mike Christie
> <michael.christie@oracle.com> wrote:
>>
>> +       if (args->worker_flags & USER_WORKER_SIG_IGN)
>> +               ignore_signals(p);
> 
> Same comment as for the other case.
> 
> There are real reasons to avoid bitfields:
> 
>  - you can't pass addresses to them around
> 
>  - it's easier to read or assign multiple fields in one go
> 
>  - they are horrible for ABI issues due to the exact bit ordering and
> padding being very subtle
> 
> but none of those issues are relevant here, where it's a kernel-internal ABI.
> 
> All these use-cases seem to actually be testing one bit at a time, and
> the "assignments" are structure initializers for which named bitfields
> are actually perfect and just make the initializer more legible.
> 

Thanks for the comments. I see what you mean and have fixed those instances and
updated kthread as well.