Re: [patch 7/7] tidy up usermode helper waiting a bit

From: Jeremy Fitzhardinge
Date: Fri May 11 2007 - 16:02:16 EST


Johannes Berg wrote:
> [/me wonders why he was CC'ed]
>

Because I knew you'd ask a good question ;) No, I looked through the
recent changes on the files I changes and guessed at likely-looking
names. But come to think of it, cc:ing completely random people is not
a bad way of getting some extra review cycles you mightn't otherwise get...


>> /* CLONE_VFORK: wait until the usermode helper has execve'd
>> * successfully We need the data structures to stay around
>> * until that is done. */
>> - if (wait)
>> + if (wait == UMH_WAIT_PROC)
>> pid = kernel_thread(wait_for_helper, sub_info,
>> CLONE_FS | CLONE_FILES | SIGCHLD);
>> else
>> pid = kernel_thread(____call_usermodehelper, sub_info,
>> CLONE_VFORK | SIGCHLD);
>>
>
> Isn't that a change in behaviour? Previously it said
> if (wait)
> <=> if (wait != 0)
> <=> if (wait != UMH_WAIT_EXEC)
> or am I missing something?
>

Hm, you're right, it is a change in behaviour. Yep, and a bug, since
its expecting wait_for_helper() to free the info in the UMH_NO_WAIT case.

---
kernel/kmod.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

===================================================================
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -243,7 +243,7 @@ static void __call_usermodehelper(struct
/* CLONE_VFORK: wait until the usermode helper has execve'd
* successfully We need the data structures to stay around
* until that is done. */
- if (wait == UMH_WAIT_PROC)
+ if (wait == UMH_WAIT_PROC || wait == UMH_NO_WAIT)
pid = kernel_thread(wait_for_helper, sub_info,
CLONE_FS | CLONE_FILES | SIGCHLD);
else

Thanks,
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/