Messages in this thread Patch in this message | | | From | Lucas De Marchi <> | Date | Tue, 12 Mar 2013 14:54:02 -0300 | Subject | Re: Regression with orderly_poweroff() |
| |
On Tue, Mar 12, 2013 at 11:46 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Mon, Mar 11, 2013 at 8:25 PM, Benjamin Herrenschmidt > <benh@kernel.crashing.org> 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;
| |