Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933172Ab3CLRyY (ORCPT ); Tue, 12 Mar 2013 13:54:24 -0400 Received: from mail-oa0-f52.google.com ([209.85.219.52]:48866 "EHLO mail-oa0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932235Ab3CLRyW (ORCPT ); Tue, 12 Mar 2013 13:54:22 -0400 MIME-Version: 1.0 In-Reply-To: References: <1363058712.4534.12.camel@pasglop> From: Lucas De Marchi Date: Tue, 12 Mar 2013 14:54:02 -0300 Message-ID: Subject: Re: Regression with orderly_poweroff() To: Linus Torvalds Cc: Benjamin Herrenschmidt , Linux Kernel Mailing List , Paul Mackerras , david@gibson.dropbear.id.au, Kees Cook , Serge Hallyn , "Rafael J. Wysocki" , Andrew Morton , Feng Hong , Oleg Nesterov , Lucas De Marchi Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2457 Lines: 68 On Tue, Mar 12, 2013 at 11:46 AM, Linus Torvalds wrote: > On Mon, Mar 11, 2013 at 8:25 PM, Benjamin Herrenschmidt > wrote: >> >> A couple of weeks ago, David sent an email that went unanswered about a >> regression concerning orderly_poweroff(). I think the original patch >> causing it should be reverted, here's the actual email with the >> explanation: > > Hmm.. You should really have cc'd the people who acked it and were in > the sign-off chain too, because all those people are involved with the > patch as well. > > Also, the patch doesn't revert cleanly any more after commit > 7ff6764061ec ("usermodehelper: cleanup/fix __orderly_poweroff() && > argv_free()") which seems to be a real bug-fix for a double free, but > which really doesn't seem to work together with UMH_NO_WAIT. Yep, using it in that way with UMH_NO_WAIT will not work. > > So before reverting that one too, let's at least get the people who > were involved with the original patch (and the bugfix that relies on > it) in the email thread. I have some pending patches on LKML to remove call_usermodehelper_fns() and export call_usermodehelper_{setup,exec}. Doing this we can separate the allocation part using GFP_ATOMIC in order to make it work on interrupt context. May I suggest going with something like below after that patches are applied (sorry, whitespace damaged)? I can also rework the patch series so this can be applied regardless of the rest. Lucas De Marchi diff --git a/kernel/sys.c b/kernel/sys.c index bd15276..6df5ec7 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -2194,7 +2194,8 @@ static int __orderly_poweroff(void) "PATH=/sbin:/bin:/usr/sbin:/usr/bin", NULL }; - int ret; + struct subprocess_info *info; + int ret = -ENOMEM; argv = argv_split(GFP_ATOMIC, poweroff_cmd, &argc); if (argv == NULL) { @@ -2203,7 +2204,10 @@ static int __orderly_poweroff(void) return -ENOMEM; } - ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); + info = call_usermodehelper_setup(argv[0], argv, envp, GFP_ATOMIC, + NULL, NULL, NULL); + if (info) + ret = call_usermodehelper_exec(info, UMH_WAIT_EXEC); argv_free(argv); return ret; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/