From: Oleg Nesterov <oleg@redhat.com>
To: Mandeep Singh Baines <msb@chromium.org>
Cc: linux-kernel@vger.kernel.org, Ben Chan <benchan@chromium.org>,
Tejun Heo <tj@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
"Rafael J. Wysocki" <rjw@sisk.pl>, Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH 5/5] coredump: ignore non-fatal signals when core dumping to a pipe
Date: Tue, 19 Feb 2013 20:45:17 +0100 [thread overview]
Message-ID: <20130219194517.GA9986@redhat.com> (raw)
In-Reply-To: <CACBanvq+Sdtv-tNXTmJyb3EvXGshC=D4NjR8aVOCjRXcTKp+Mw@mail.gmail.com>
On 02/19, Mandeep Singh Baines wrote:
>
> On Tue, Feb 19, 2013 at 6:27 AM, Oleg Nesterov <oleg@redhat.com> wrote:
> > Please look at 1-3 I sent. Btw, I slightly tested this series, seems
> > to work...
> >
>
> They look good to me. I plan on applying them to our tree since we
> need a fix ASAP.
Great!
> >> You'd need to prevent the fake signal from freeezer from setting
> >> TIF_SIGPENDING. Maybe just add a SIGNAL_GROUP_EXIT check in freezer.c.
> >
> > I am thinking about checking SIGNAL_GROUP_COREDUMP but I am not sure,
> > perhaps we can make a simpler solution. As for wait_for_dump_helper()
> > we do not need any check at all, but we should either fix
> > wait_event_freezable (it is actually not right) or change pipe_release()
>
> Is the bug that it will exit on the fake_signal.
Yes, I understand, but
> I don't think that bug will affects this patch though. I think this
> should all work if we add a check to freezer.c (or something similar
> that is cleaner).
>
> If you add SIGNAL_GROUP_COREDUMP check to freezer.c:
>
> static void fake_signal_wake_up(struct task_struct *p)
> {
> unsigned long flags;
>
> if (lock_task_sighand(p, &flags)) {
> - signal_wake_up(p, 0);
> + if (!p->signal->flags & SIGNAL_GROUP_COREDUMP)
> + signal_wake_up(p, 0);
> unlock_task_sighand(p, &flags);
> }
> }
>
> And change the wait_event_freezekillable() in this patch to just
> wait_event_freezable(), shouldn't that just work.
I doubt,
> The fake signal will never get sent.
Yes but try_to_freeze_tasks() can fail.
And once again, if wait_event_freezable() was correct we do not care
about the fake signal (in wait_for_dump_helper), so we do not need
to change fake_signal_wake_up. So perhaps we should fix it but this
needs some discussion.
Sorry again for the terse reply (and perhaps I misunderstood you),
I'll try to return to this problem asap. In any case I still think
we should do the freezer fixes on top of signal fixes I sent, and
you seem to agree. Good ;)
Oleg.
next prev parent reply other threads:[~2013-02-19 19:46 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-16 9:53 [PATCH 1/5] vfork: don't freezer_count() for in-kernel users of CLONE_VFORK Mandeep Singh Baines
2013-02-16 9:53 ` [PATCH 2/5] lockdep: check that no locks held at freeze time Mandeep Singh Baines
2013-02-16 17:06 ` Oleg Nesterov
2013-02-20 1:57 ` [PATCH v3] " Mandeep Singh Baines
2013-02-20 10:37 ` Ingo Molnar
2013-02-20 12:55 ` Rafael J. Wysocki
2013-02-20 13:52 ` Ingo Molnar
2013-02-20 22:30 ` Oleg Nesterov
2013-02-20 23:11 ` Mandeep Singh Baines
2013-02-20 23:17 ` [PATCH v4] " Mandeep Singh Baines
2013-02-20 23:24 ` Andrew Morton
2013-02-21 0:17 ` Mandeep Singh Baines
2013-02-21 0:20 ` Andrew Morton
2013-02-21 0:28 ` Mandeep Singh Baines
2013-02-21 0:42 ` Andrew Morton
2013-02-21 3:19 ` Mandeep Singh Baines
2013-02-21 3:17 ` [PATCH v5] " Mandeep Singh Baines
2013-02-21 15:42 ` Rafael J. Wysocki
2013-02-21 16:24 ` Mandeep Singh Baines
2013-02-21 16:51 ` [PATCH v6] " Mandeep Singh Baines
2013-02-21 21:42 ` Andrew Morton
2013-02-21 21:57 ` Mandeep Singh Baines
2013-02-16 9:53 ` [PATCH 3/5] coredump: use a freezable_schedule for the coredump_finish wait Mandeep Singh Baines
2013-02-16 17:17 ` Oleg Nesterov
2013-02-16 9:53 ` [PATCH 4/5] freezer: clear fake signal on exit from __refrigerator Mandeep Singh Baines
2013-02-16 17:06 ` Oleg Nesterov
2013-02-16 17:12 ` Oleg Nesterov
2013-02-20 18:09 ` Mandeep Singh Baines
2013-02-23 19:41 ` Oleg Nesterov
2013-02-23 19:59 ` Oleg Nesterov
2013-02-16 9:53 ` [PATCH 5/5] coredump: ignore non-fatal signals when core dumping to a pipe Mandeep Singh Baines
2013-02-16 17:10 ` Oleg Nesterov
2013-02-16 19:46 ` Oleg Nesterov
2013-02-18 23:55 ` Mandeep Singh Baines
2013-02-19 14:18 ` Oleg Nesterov
2013-02-19 5:19 ` Mandeep Singh Baines
2013-02-19 14:27 ` Oleg Nesterov
2013-02-19 19:33 ` Mandeep Singh Baines
2013-02-19 19:45 ` Oleg Nesterov [this message]
2013-02-19 20:20 ` Mandeep Singh Baines
2013-02-20 23:30 ` Mandeep Singh Baines
2013-02-23 19:21 ` Oleg Nesterov
2013-02-16 17:05 ` [PATCH 1/5] vfork: don't freezer_count() for in-kernel users of CLONE_VFORK Oleg Nesterov
2013-02-20 0:07 ` Mandeep Singh Baines
2013-02-20 1:41 ` Mandeep Singh Baines
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130219194517.GA9986@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=benchan@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=msb@chromium.org \
--cc=rjw@sisk.pl \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.