Messages in this thread | | | Date | Wed, 28 May 2008 09:05:28 +0100 | From | Jeremy Fitzhardinge <> | Subject | Re: [PATCH] call_usermodehelper() increase reliability |
| |
KOSAKI Motohiro wrote: > Now, call_usermodehelper_setup() uses GFP_ATOMIC. > but it can return NULL _very_ easily. > > GFP_ATOMIC is needed only when we can't sleep. > and, GFP_KERNEL is robust and better. > > thus, I add gfp_mask argument to call_usermodehelper_setup(). > > So, its caller pass below, > > call_usermodehelper() and call_usermodehelper_keys(): > depend on 'wait' argument. > call_usermodehelper_pipe(): > always GFP_KERNEL because always run under process context. > orderly_poweroff(): > pass to GFP_ATOMIC because may run under interrupt context. > > > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > CC: "Paul Menage" <menage@google.com> > CC: Li Zefan <lizf@cn.fujitsu.com> > CC: Jeremy Fitzhardinge <jeremy@xensource.com> > CC: Rusty Russell <rusty@rustcorp.com.au> > CC: Andi Kleen <andi@firstfloor.org> >
Yes, looks good.
Acked-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> --- > include/linux/kmod.h | 11 +++++++---- > kernel/kmod.c | 9 +++++---- > kernel/sys.c | 2 +- > 3 files changed, 13 insertions(+), 9 deletions(-) > > Index: b/kernel/kmod.c > =================================================================== > --- a/kernel/kmod.c > +++ b/kernel/kmod.c > @@ -352,16 +352,17 @@ static inline void register_pm_notifier_ > * @path: path to usermode executable > * @argv: arg vector for process > * @envp: environment for process > + * @gfp_mask: gfp mask for memory allocation > * > * Returns either %NULL on allocation failure, or a subprocess_info > * structure. This should be passed to call_usermodehelper_exec to > * exec the process and free the structure. > */ > -struct subprocess_info *call_usermodehelper_setup(char *path, > - char **argv, char **envp) > +struct subprocess_info *call_usermodehelper_setup(char *path, char **argv, > + char **envp, gfp_t gfp_mask) > { > struct subprocess_info *sub_info; > - sub_info = kzalloc(sizeof(struct subprocess_info), GFP_ATOMIC); > + sub_info = kzalloc(sizeof(struct subprocess_info), gfp_mask); > if (!sub_info) > goto out; > > @@ -494,7 +495,7 @@ int call_usermodehelper_pipe(char *path, > struct subprocess_info *sub_info; > int ret; > > - sub_info = call_usermodehelper_setup(path, argv, envp); > + sub_info = call_usermodehelper_setup(path, argv, envp, GFP_KERNEL); > if (sub_info == NULL) > return -ENOMEM; > > Index: b/include/linux/kmod.h > =================================================================== > --- a/include/linux/kmod.h > +++ b/include/linux/kmod.h > @@ -19,6 +19,7 @@ > * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. > */ > > +#include <linux/gfp.h> > #include <linux/stddef.h> > #include <linux/errno.h> > #include <linux/compiler.h> > @@ -40,8 +41,8 @@ struct file; > struct subprocess_info; > > /* Allocate a subprocess_info structure */ > -struct subprocess_info *call_usermodehelper_setup(char *path, > - char **argv, char **envp); > +struct subprocess_info *call_usermodehelper_setup(char *path, char **argv, > + char **envp, gfp_t gfp_mask); > > /* Set various pieces of state into the subprocess_info structure */ > void call_usermodehelper_setkeys(struct subprocess_info *info, > @@ -68,8 +69,9 @@ static inline int > call_usermodehelper(char *path, char **argv, char **envp, enum umh_wait wait) > { > struct subprocess_info *info; > + gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; > > - info = call_usermodehelper_setup(path, argv, envp); > + info = call_usermodehelper_setup(path, argv, envp, gfp_mask); > if (info == NULL) > return -ENOMEM; > return call_usermodehelper_exec(info, wait); > @@ -80,8 +82,9 @@ call_usermodehelper_keys(char *path, cha > struct key *session_keyring, enum umh_wait wait) > { > struct subprocess_info *info; > + gfp_t gfp_mask = (wait == UMH_NO_WAIT) ? GFP_ATOMIC : GFP_KERNEL; > > - info = call_usermodehelper_setup(path, argv, envp); > + info = call_usermodehelper_setup(path, argv, envp, gfp_mask); > if (info == NULL) > return -ENOMEM; > > Index: b/kernel/sys.c > =================================================================== > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -1795,7 +1795,7 @@ int orderly_poweroff(bool force) > goto out; > } > > - info = call_usermodehelper_setup(argv[0], argv, envp); > + info = call_usermodehelper_setup(argv[0], argv, envp, GFP_ATOMIC); > if (info == NULL) { > argv_free(argv); > goto out; > > > -- > 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/ >
| |