Messages in this thread | | | Date | Wed, 24 Oct 2018 16:02:15 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH v3] kernel/signal: Signal-based pre-coredump notification |
| |
On 10/23, Enke Chen wrote: > > --- a/fs/coredump.c > +++ b/fs/coredump.c > @@ -590,6 +590,12 @@ void do_coredump(const kernel_siginfo_t *siginfo) > if (retval < 0) > goto fail_creds; > > + /* > + * Send the pre-coredump signal to the parent if requested. > + */ > + do_notify_parent_predump(); > + cond_resched();
I am still not sure cond_resched() makes any sense here...
> @@ -1553,6 +1553,9 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk) > tty_audit_fork(sig); > sched_autogroup_fork(sig); > > + /* Clear the pre-coredump signal for the child */ > + sig->predump_signal = 0;
No need, copy_signal() does zalloc().
> +void do_notify_parent_predump(void) > +{ > + struct sighand_struct *sighand; > + struct kernel_siginfo info; > + struct task_struct *parent; > + unsigned long flags; > + int sig; > + > + read_lock(&tasklist_lock); > + parent = current->parent; > + sig = parent->signal->predump_signal; > + if (sig != 0) { > + clear_siginfo(&info); > + info.si_pid = task_tgid_vnr(current); > + info.si_signo = sig; > + if (sig == SIGCHLD) > + info.si_code = CLD_PREDUMP; > + > + sighand = parent->sighand; > + spin_lock_irqsave(&sighand->siglock, flags); > + __group_send_sig_info(sig, &info, parent); > + spin_unlock_irqrestore(&sighand->siglock, flags);
You can just use do_send_sig_info() and remove sighand/flags/spin_lock_irqsave.
Perhaps the "likely" predump_signal==0 check at the start makes sense to avoid read_lock(tasklist).
And I'd suggest to move it into coredump.c and make it static. It won't have another user.
Oleg.
| |