On 02/16, Mandeep Singh Baines wrote:
>
> Make wait_for_dump_helpers() not abort piping the core dump data when the
> crashing process has received a non-fatal signal. The abort still occurs
> in the case of SIGKILL.
>
> Testing:
>
> localhost ~ # echo "|/usr/bin/sleep 1d" > /proc/sys/kernel/core_pattern
> localhost ~ # sleep 1d &
As I already said, this is not enough. And if we change send_signal() paths
to "ignore" the non-fatal signals sent to the dumping process (and I think
we should do this anyway), this change is not needed.
Except we have other problems with the freezer.
> +static int sigkill_pending(struct task_struct *tsk)
> +{
> + return signal_pending(tsk) &&
> + (sigismember(&tsk->pending.signal, SIGKILL) ||
> + sigismember(&tsk->signal->shared_pending.signal, SIGKILL));
> +}
Why? __fatal_signal_pending() is enough, you do not need to check
->shared_pending. And once again, ignoring the freezer problems I
do not think we need this check at all.
IOW. Yes, we will probably need to do this change but only to be
freezer-friendly.
> static void wait_for_dump_helpers(struct file *file)
> {
> struct pipe_inode_info *pipe;
> + sigset_t blocked, previous;
> +
> + /* Block all but fatal signals. */
> + siginitsetinv(&blocked, sigmask(SIGKILL));
> + sigprocmask(SIG_BLOCK, &blocked, &previous);
(sigprocmask() must die, please never use, we have set_current_blocked().
Although in this particular case this doesn't matter...)
Heh. When I suggested this change a looong ago, my attempt was NACK'ed
because the core handler looks at /proc/pid/status.
If we could do this, we could simply ignore all signals except SIGKILL
at the start, in zap_threads(). This could solve almost all problems
with the signals/SIGKILL.
But see above, we can't.
Anyway. Please look at the patch below. I need to recheck it, and I was
going to send it later, along with other changes I am _trying_ to do. But
if it is correct, it looks certainly better and perhaps it can go ahead.
Afaics it could equally fix the mentioned problems (again, if correct ;).
"equally" also means it is equally incomplete.
Oleg.
--- x/fs/coredump.c
+++ x/fs/coredump.c
@@ -416,17 +416,17 @@ static void wait_for_dump_helpers(struct
pipe_lock(pipe);
pipe->readers++;
pipe->writers--;
+ // TODO: wake_up_interruptible_sync_poll ?
+ wake_up_interruptible_sync(&pipe->wait);
+ kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
+ pipe_unlock(pipe);
- while ((pipe->readers > 1) && (!signal_pending(current))) {
- wake_up_interruptible_sync(&pipe->wait);
- kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
- pipe_wait(pipe);
- }
+ wait_event_freezekillable(&pipe->wait, pipe->readers == 1);
+ pipe_lock(pipe);
pipe->readers--;
pipe->writers++;
pipe_unlock(pipe);
-
}
/*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
