Re: [PATCH] call_usermodehelper_setup() should use GFP_KERNEL

From: Jeremy Fitzhardinge
Date: Thu May 08 2008 - 08:58:30 EST


KOSAKI Motohiro wrote:
Hi

Thanks good comment!

How many times do we have to make this mistake :(

Only the caller knows what allocation mode the callee can use. call_usermodehelper_setup() should be extended to take a gfp_t argument.
Yeah, but making the caller need to know about the internal implementation details of the callee (ie, whether it needs to allocate memory or not) leads to pretty warty interfaces. In this case, you could push the gfp_t up to the call_usermodehelper_setup() level, but pushing it any higher wouldn't make much sense.

No problem :)
almost caller doesn't call call_usermodehelper_setup() directly.

thus, call_usermodehelper_setup() chage is hided in call_usermodehelper().

Yep, seems reasonable. Are there any UMH_NO_WAIT callers who could be using GFP_KERNEL?

----------------chunk of my current testing patch-----------------------------

@@ -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);


J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/