From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933674Ab3BSUUI (ORCPT ); Tue, 19 Feb 2013 15:20:08 -0500 Received: from mail-oa0-f46.google.com ([209.85.219.46]:43579 "EHLO mail-oa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933529Ab3BSUUF (ORCPT ); Tue, 19 Feb 2013 15:20:05 -0500 MIME-Version: 1.0 In-Reply-To: <20130219194517.GA9986@redhat.com> References: <1361008406-2307-1-git-send-email-msb@chromium.org> <1361008406-2307-5-git-send-email-msb@chromium.org> <20130216171010.GE4910@redhat.com> <20130216194643.GA31569@redhat.com> <20130219142721.GA5709@redhat.com> <20130219194517.GA9986@redhat.com> Date: Tue, 19 Feb 2013 12:20:04 -0800 X-Google-Sender-Auth: 9kSGaRrVgU4IlJ3GtqCIIqIfCfc Message-ID: Subject: Re: [PATCH 5/5] coredump: ignore non-fatal signals when core dumping to a pipe From: Mandeep Singh Baines To: Oleg Nesterov Cc: linux-kernel@vger.kernel.org, Ben Chan , Tejun Heo , Andrew Morton , "Rafael J. Wysocki" , Ingo Molnar Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 19, 2013 at 11:45 AM, Oleg Nesterov wrote: > On 02/19, Mandeep Singh Baines wrote: >> >> On Tue, Feb 19, 2013 at 6:27 AM, Oleg Nesterov 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. > Ah. Good point. How about this then: /* can't use wait_event_freezable since we suppress the fake signal on SIGNAL_GROUP_COREDUMP */ freezer_do_not_count(); wait_event_interruptible(pipe->wait, pipe->readers == 1); freezer_count(); Regards, Mandeep > 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. >