[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <E62404B31D39064FBDD0B843ADF05A7D2D395FCD2F@SC-VEXCH4.marvell.com>
Date: Thu, 20 Sep 2012 17:16:35 -0700
From: Feng Hong <hongfeng@...vell.com>
To: "Serge E. Hallyn" <serge@...lyn.com>
CC: "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"gorcunov@...nvz.org" <gorcunov@...nvz.org>,
"keescook@...omium.org" <keescook@...omium.org>,
"serge.hallyn@...onical.com" <serge.hallyn@...onical.com>,
"ebiederm@...ssion.com" <ebiederm@...ssion.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH V2] poweroff: fix bug in orderly_poweroff
Hi, Serge,
I am just a graduate and it's my first time to send a patch to opensource, so thank you very much for reminding me the "changelog affairs", it seems this patch has been added to -mm tree as attached mail, and I have no chance to change the comments, right ? Then I must remember this and be careful next time. Thanks again for reminding me !
>Is this actually sufficient for you? The exec will have started, but may for whatever (very unlikely) reason fail. If you're happy with it,
I think UMH_WAIT_EXEC is sufficient for me, as in our system there is no "/sbin/poweroff" existed. On the other hand, UMH_WAIT_PROC is not suitable here as Eric analysis; if using UMH_WAIT_EXEC, and the user application fail, I'd prefer to complain bad application. So using UMH_WAIT_EXEC and UMH_WAIT_PROC has a tradeoff here, what do you think so ?
--
Best Regards,
Feng Hong
Application Processor Software Engnieer
Marvell Technology (Shanghai) Ltd
-----Original Message-----
From: Serge E. Hallyn [mailto:serge@...lyn.com]
Sent: 2012年9月21日 1:07
To: Feng Hong
Cc: akpm@...ux-foundation.org; gorcunov@...nvz.org; keescook@...omium.org; serge.hallyn@...onical.com; ebiederm@...ssion.com; linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] poweroff: fix bug in orderly_poweroff
Quoting hongfeng (hongfeng@...vell.com):
> orderly_poweroff is trying to poweroff platform by two steps:
> step 1: Call userspace application to poweroff
> step 2: If userspace poweroff fail, then do a force power off if force param is set.
>
> The bug here is, step 1 is always successful with param UMH_NO_WAIT,
Note that a changelog here explaining that you switched to UMH_WAIT_EXEC
per Eric's suggestion would be both informative and courteous.
> should change to UMH_WAIT_EXEC which will monitor whether user application successful run.
Is this actually sufficient for you? The exec will have started, but
may for whatever (very unlikely) reason fail. If you're happy with
it,
Acked-by: Serge Hallyn <serge.hallyn@...onical.com>
> Change-Id: I2f9ebbb90c0c2443780080ec9507c8d004e5da74
> Signed-off-by: Feng Hong <hongfeng@...vell.com>
> Acked-by: Kees Cook <keescook@...omium.org>
> ---
> kernel/sys.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 241507f..a624d4c 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -2204,7 +2204,7 @@ static int __orderly_poweroff(void)
> return -ENOMEM;
> }
>
> - ret = call_usermodehelper_fns(argv[0], argv, envp, UMH_NO_WAIT,
> + ret = call_usermodehelper_fns(argv[0], argv, envp, UMH_WAIT_EXEC,
> NULL, argv_cleanup, NULL);
> if (ret == -ENOMEM)
> argv_free(argv);
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
Received: from psmtp.com (74.125.149.98) by sc-owa01.marvell.com (10.93.76.21)
with Microsoft SMTP Server id 8.3.213.0; Wed, 19 Sep 2012 12:12:43 -0700
Received: from mail-bk0-f74.google.com ([209.85.214.74]) (using TLSv1) by
na3sys009amx172.postini.com ([74.125.148.10]) with SMTP; Wed, 19 Sep 2012
12:12:43 PDT
Received: by bkcjc3 with SMTP id jc3so77872bkc.5 for
<hongfeng@...vell.com>; Wed, 19 Sep 2012 12:12:41 -0700 (PDT)
Received: by 10.180.106.199 with SMTP id gw7mr1144783wib.0.1348081961502;
Wed, 19 Sep 2012 12:12:41 -0700 (PDT)
Received: from hpza10.eem.corp.google.com ([74.125.121.33]) by
gmr-mx.google.com with ESMTPS id fb20si2597379wid.3.2012.09.19.12.12.41
(version=TLSv1/SSLv3 cipher=AES128-SHA); Wed, 19 Sep 2012
12:12:41 -0700 (PDT)
Received: from localhost.localdomain (akpm.mtv.corp.google.com [172.18.96.75])
by hpza10.eem.corp.google.com (Postfix) with ESMTP id 2DDC120004E; Wed, 19
Sep 2012 12:12:40 -0700 (PDT)
From: "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>
To: "mm-commits@...r.kernel.org" <mm-commits@...r.kernel.org>
CC: Feng Hong <hongfeng@...vell.com>, "ebiederm@...ssion.com"
<ebiederm@...ssion.com>, "keescook@...omium.org" <keescook@...omium.org>,
"rjw@...k.pl" <rjw@...k.pl>, "serge.hallyn@...onical.com"
<serge.hallyn@...onical.com>
Date: Wed, 19 Sep 2012 12:12:39 -0700
Subject: + poweroff-fix-bug-in-orderly_poweroff.patch added to -mm tree
Thread-Topic: + poweroff-fix-bug-in-orderly_poweroff.patch added to -mm tree
Thread-Index: Ac2Wmr8bdW2D4x2bSA+9H7NqJvO+rw==
Message-ID: <20120919191240.2DDC120004E@...a10.eem.corp.google.com>
X-MS-Exchange-Organization-AuthAs: Anonymous
X-MS-Exchange-Organization-AuthSource: SC-OWA01.marvell.com
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-pstn-settings: 3 (1.0000:0.0100) s cv GT3 gt2 gt1 r p m c
x-pstn-levels: (S:99.90000/99.90000 CV:99.9000 FC:95.5390 LC:95.5390
R:95.9108 P:95.9108 M:97.0282 C:98.6951 )
x-pstn-addresses: from <akpm@...ux-foundation.org> [db-null]
x-pstn-neptune: 0/0/0.00/0
x-pstn-dkim: 0 skipped:not-enabled
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
The patch titled
Subject: poweroff: fix bug in orderly_poweroff()
has been added to the -mm tree. Its filename is
poweroff-fix-bug-in-orderly_poweroff.patch
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/SubmitChecklist when testing your code **=
*
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: hongfeng <hongfeng@...vell.com>
Subject: poweroff: fix bug in orderly_poweroff()
orderly_poweroff is trying to poweroff platform by two steps:
step 1: Call userspace application to poweroff
step 2: If userspace poweroff fail, then do a force power off if force
param is set.
The bug here is, step 1 is always successful with param UMH_NO_WAIT,
should change to UMH_WAIT_EXEC which will monitor whether user application
successful run.
Signed-off-by: Feng Hong <hongfeng@...vell.com>
Acked-by: Kees Cook <keescook@...omium.org>
Cc: Serge Hallyn <serge.hallyn@...onical.com>
Cc: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: "Rafael J. Wysocki" <rjw@...k.pl>
Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
---
kernel/sys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -puN kernel/sys.c~poweroff-fix-bug-in-orderly_poweroff kernel/sys.c
--- a/kernel/sys.c~poweroff-fix-bug-in-orderly_poweroff
+++ a/kernel/sys.c
@@ -2205,7 +2205,7 @@ static int __orderly_poweroff(void)
return -ENOMEM;
}
- ret =3D call_usermodehelper_fns(argv[0], argv, envp, UMH_NO_WAIT,
+ ret =3D call_usermodehelper_fns(argv[0], argv, envp, UMH_WAIT_EXEC,
NULL, argv_cleanup, NULL);
if (ret =3D=3D -ENOMEM)
argv_free(argv);
_
Patches currently in -mm which might be from hongfeng@...vell.com are
poweroff-fix-bug-in-orderly_poweroff.patch
Powered by blists - more mailing lists