if we use a kref instead of xchg to govern the lifetime
of struct subprocess_info, the code becomes simpler and
more generic. We can avoid some of the special cases.
Actually I like the xchg use here. But if it will break
some ARCHs that do not like xchg, then here is the work
needed.
No functional change
CC: Oleg Nesterov <oleg@xxxxxxxxxx>
Signed-off-by: Boaz Harrosh <bharrosh@xxxxxxxxxxx>
---
include/linux/kmod.h | 4 +++-
kernel/kmod.c | 43 +++++++++++++++++--------------------------
2 files changed, 20 insertions(+), 27 deletions(-)
diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index 4d9f202..bd0c3c9 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -25,6 +25,7 @@
#include <linux/compiler.h>
#include <linux/workqueue.h>
#include <linux/sysctl.h>
+#include <linux/kref.h>
#define KMOD_PATH_LEN 256
@@ -54,8 +55,9 @@ extern __printf(2, 3)
#define UMH_KILLABLE 4 /* wait for EXEC/PROC killable */
struct subprocess_info {
+ struct kref kref;
struct work_struct work;
- struct completion *complete;
+ struct completion complete;
char *path;
char **argv;
char **envp;
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 588cb6f..948a674 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -221,9 +221,11 @@ static int ____call_usermodehelper(void *data)
return 0;
}
-static
-void call_usermodehelper_freeinfo(struct subprocess_info *info)
+static void call_usermodehelper_freeinfo(struct kref *kref)
{
+ struct subprocess_info *info =
+ container_of(kref, struct subprocess_info, kref);
+
if (info->cleanup)
(*info->cleanup)(info);
kfree(info);
@@ -231,15 +233,8 @@ void call_usermodehelper_freeinfo(struct subprocess_info *info)
static void umh_complete(struct subprocess_info *sub_info)
{
- struct completion *comp = xchg(&sub_info->complete, NULL);
- /*
- * See call_usermodehelper_exec(). If xchg() returns NULL
- * we own sub_info, the UMH_KILLABLE caller has gone away.
- */
- if (comp)
- complete(comp);
- else
- call_usermodehelper_freeinfo(sub_info);
+ complete(&sub_info->complete);
+ kref_put(&sub_info->kref, call_usermodehelper_freeinfo);
}
/* Keventd can't block, but this (a child) can. */
@@ -302,7 +297,7 @@ static void __call_usermodehelper(struct work_struct *work)
switch (wait) {
case UMH_NO_WAIT:
- call_usermodehelper_freeinfo(sub_info);
+ kref_put(&sub_info->kref, call_usermodehelper_freeinfo);
break;
case UMH_WAIT_PROC:
@@ -431,6 +426,8 @@ struct subprocess_info *call_usermodehelper_setup(char *path, char **argv,
if (!sub_info)
goto out;
+ kref_init(&sub_info->kref);
+ init_completion(&sub_info->complete);
INIT_WORK(&sub_info->work, __call_usermodehelper);
sub_info->path = path;
sub_info->argv = argv;
@@ -521,7 +518,6 @@ void call_usermodehelper_setfns(struct subprocess_info *info,
static
int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait)
{
- DECLARE_COMPLETION_ONSTACK(done);
int wait_state;
int retval = 0;
@@ -534,26 +530,21 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait)
goto out;
}
- sub_info->complete = &done;
sub_info->wait = wait;
+ /* Balanced in __call_usermodehelper or wait_for_helper */
+ kref_get(&sub_info->kref);
queue_work(khelper_wq, &sub_info->work);
if (wait == UMH_NO_WAIT) /* task has freed sub_info */
- goto unlock;
+ goto out;
wait_state = (wait & UMH_KILLABLE) ? TASK_KILLABLE : 0;
- retval = wait_for_completion_timeout_state(&done, sub_info->timeout,
- wait_state);
- if (unlikely(retval)) {
- /* umh_complete() will see NULL and free sub_info */
- if (xchg(&sub_info->complete, NULL))
- goto unlock;
- }
-
- retval = sub_info->retval;
+ retval = wait_for_completion_timeout_state(&sub_info->complete,
+ sub_info->timeout, wait_state);
+ if (likely(!retval))
+ retval = sub_info->retval;
out:
- call_usermodehelper_freeinfo(sub_info);
-unlock:
+ kref_put(&sub_info->kref, call_usermodehelper_freeinfo);
helper_unlock();
return retval;
}
--
1.7.6.5
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
