WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [patch 05/44] usermodehelper: Tidy up waiting

To: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [patch 05/44] usermodehelper: Tidy up waiting
From: Jeremy Fitzhardinge <jeremy@xxxxxxxxxxxxx>
Date: Mon, 16 Jul 2007 16:15:41 -0700
Cc: Randy Dunlap <randy.dunlap@xxxxxxxxxx>, Christoph Hellwig <hch@xxxxxxxxxxxxx>, Jeremy Fitzhardinge <jeremy@xxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Tony Luck <tony.luck@xxxxxxxxx>, Srivatsa Vaddagiri <vatsa@xxxxxxxxxx>, Kay Sievers <kay.sievers@xxxxxxxx>, Andi Kleen <ak@xxxxxxx>, Ralf Baechle <ralf@xxxxxxxxxxxxxx>, lkml <linux-kernel@xxxxxxxxxxxxxxx>, James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, Paul Mackerras <paulus@xxxxxxxxx>, Joel Becker <joel.becker@xxxxxxxxxx>, David Howells <dhowells@xxxxxxxxxx>, Johannes Berg <johannes@xxxxxxxxxxxxxxxx>, Oleg Nesterov <oleg@xxxxxxxxxx>, Bjorn Helgaas <bjorn.helgaas@xxxxxx>
Delivery-date: Mon, 16 Jul 2007 16:56:42 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20070716231536.937393000@xxxxxxxxxxxxx>>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: quilt/0.46-1
Rather than using a tri-state integer for the wait flag in
call_usermodehelper_exec, define a proper enum, and use that.  I've
preserved the integer values so that any callers I've missed should
still work OK.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xxxxxxxxxxxxx>
Cc: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
Cc: Randy Dunlap <randy.dunlap@xxxxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Cc: Andi Kleen <ak@xxxxxxx>
Cc: Paul Mackerras <paulus@xxxxxxxxx>
Cc: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
Cc: Bjorn Helgaas <bjorn.helgaas@xxxxxx>
Cc: Joel Becker <joel.becker@xxxxxxxxxx>
Cc: Tony Luck <tony.luck@xxxxxxxxx>
Cc: Kay Sievers <kay.sievers@xxxxxxxx>
Cc: Srivatsa Vaddagiri <vatsa@xxxxxxxxxx>
Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
Cc: David Howells <dhowells@xxxxxxxxxx>
---
 arch/i386/mach-voyager/voyager_thread.c |    2 +-
 arch/x86_64/kernel/mce.c                |    2 +-
 drivers/macintosh/therm_pm72.c          |    3 ++-
 drivers/macintosh/windfarm_core.c       |    3 ++-
 drivers/net/hamradio/baycom_epp.c       |    2 +-
 drivers/pnp/pnpbios/core.c              |    2 +-
 fs/ocfs2/heartbeat.c                    |    2 +-
 include/linux/kmod.h                    |   12 +++++++++---
 kernel/cpuset.c                         |    2 +-
 kernel/kmod.c                           |   29 +++++++++++++++++------------
 kernel/sys.c                            |    2 +-
 lib/kobject_uevent.c                    |    2 +-
 net/bridge/br_stp_if.c                  |    2 +-
 security/keys/request_key.c             |    3 ++-
 14 files changed, 41 insertions(+), 27 deletions(-)

===================================================================
--- a/arch/i386/mach-voyager/voyager_thread.c
+++ b/arch/i386/mach-voyager/voyager_thread.c
@@ -52,7 +52,7 @@ execute(const char *string)
                NULL,
        };
 
-       if ((ret = call_usermodehelper(argv[0], argv, envp, 1)) != 0) {
+       if ((ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC)) != 
0) {
                printk(KERN_ERR "Voyager failed to run \"%s\": %i\n",
                       string, ret);
        }
===================================================================
--- a/arch/x86_64/kernel/mce.c
+++ b/arch/x86_64/kernel/mce.c
@@ -174,7 +174,7 @@ static void do_mce_trigger(void)
        if (events != atomic_read(&mce_logged) && trigger[0]) {
                /* Small race window, but should be harmless.  */
                atomic_set(&mce_logged, events);
-               call_usermodehelper(trigger, trigger_argv, NULL, -1);
+               call_usermodehelper(trigger, trigger_argv, NULL, UMH_NO_WAIT);
        }
 }
 
===================================================================
--- a/drivers/macintosh/therm_pm72.c
+++ b/drivers/macintosh/therm_pm72.c
@@ -1770,7 +1770,8 @@ static int call_critical_overtemp(void)
                                "PATH=/sbin:/usr/sbin:/bin:/usr/bin",
                                NULL };
 
-       return call_usermodehelper(critical_overtemp_path, argv, envp, 0);
+       return call_usermodehelper(critical_overtemp_path,
+                                  argv, envp, UMH_WAIT_EXEC);
 }
 
 
===================================================================
--- a/drivers/macintosh/windfarm_core.c
+++ b/drivers/macintosh/windfarm_core.c
@@ -80,7 +80,8 @@ int wf_critical_overtemp(void)
                                "PATH=/sbin:/usr/sbin:/bin:/usr/bin",
                                NULL };
 
-       return call_usermodehelper(critical_overtemp_path, argv, envp, 0);
+       return call_usermodehelper(critical_overtemp_path,
+                                  argv, envp, UMH_WAIT_EXEC);
 }
 EXPORT_SYMBOL_GPL(wf_critical_overtemp);
 
===================================================================
--- a/drivers/net/hamradio/baycom_epp.c
+++ b/drivers/net/hamradio/baycom_epp.c
@@ -320,7 +320,7 @@ static int eppconfig(struct baycom_state
        sprintf(portarg, "%ld", bc->pdev->port->base);
        printk(KERN_DEBUG "%s: %s -s -p %s -m %s\n", bc_drvname, 
eppconfig_path, portarg, modearg);
 
-       return call_usermodehelper(eppconfig_path, argv, envp, 1);
+       return call_usermodehelper(eppconfig_path, argv, envp, UMH_WAIT_PROC);
 }
 
 /* ---------------------------------------------------------------------- */
===================================================================
--- a/drivers/pnp/pnpbios/core.c
+++ b/drivers/pnp/pnpbios/core.c
@@ -147,7 +147,7 @@ static int pnp_dock_event(int dock, stru
                info->location_id, info->serial, info->capabilities);
        envp[i] = NULL;
        
-       value = call_usermodehelper (argv [0], argv, envp, 0);
+       value = call_usermodehelper (argv [0], argv, envp, UMH_WAIT_EXEC);
        kfree (buf);
        kfree (envp);
        return 0;
===================================================================
--- a/fs/ocfs2/heartbeat.c
+++ b/fs/ocfs2/heartbeat.c
@@ -209,7 +209,7 @@ void ocfs2_stop_heartbeat(struct ocfs2_s
        envp[1] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
        envp[2] = NULL;
 
-       ret = call_usermodehelper(argv[0], argv, envp, 1);
+       ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
        if (ret < 0)
                mlog_errno(ret);
 }
===================================================================
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -51,15 +51,21 @@ void call_usermodehelper_setcleanup(stru
 void call_usermodehelper_setcleanup(struct subprocess_info *info,
                                    void (*cleanup)(char **argv, char **envp));
 
+enum umh_wait {
+       UMH_NO_WAIT = -1,       /* don't wait at all */
+       UMH_WAIT_EXEC = 0,      /* wait for the exec, but not the process */
+       UMH_WAIT_PROC = 1,      /* wait for the process to complete */
+};
+
 /* Actually execute the sub-process */
-int call_usermodehelper_exec(struct subprocess_info *info, int wait);
+int call_usermodehelper_exec(struct subprocess_info *info, enum umh_wait wait);
 
 /* Free the subprocess_info. This is only needed if you're not going
    to call call_usermodehelper_exec */
 void call_usermodehelper_freeinfo(struct subprocess_info *info);
 
 static inline int
-call_usermodehelper(char *path, char **argv, char **envp, int wait)
+call_usermodehelper(char *path, char **argv, char **envp, enum umh_wait wait)
 {
        struct subprocess_info *info;
 
@@ -71,7 +77,7 @@ call_usermodehelper(char *path, char **a
 
 static inline int
 call_usermodehelper_keys(char *path, char **argv, char **envp,
-                        struct key *session_keyring, int wait)
+                        struct key *session_keyring, enum umh_wait wait)
 {
        struct subprocess_info *info;
 
===================================================================
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -516,7 +516,7 @@ static void cpuset_release_agent(const c
        envp[i++] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
        envp[i] = NULL;
 
-       call_usermodehelper(argv[0], argv, envp, 0);
+       call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
        kfree(pathbuf);
 }
 
===================================================================
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -119,7 +119,7 @@ struct subprocess_info {
        char **argv;
        char **envp;
        struct key *ring;
-       int wait;
+       enum umh_wait wait;
        int retval;
        struct file *stdin;
        void (*cleanup)(char **argv, char **envp);
@@ -225,7 +225,7 @@ static int wait_for_helper(void *data)
                        sub_info->retval = ret;
        }
 
-       if (sub_info->wait < 0)
+       if (sub_info->wait == UMH_NO_WAIT)
                call_usermodehelper_freeinfo(sub_info);
        else
                complete(sub_info->complete);
@@ -238,26 +238,31 @@ static void __call_usermodehelper(struct
        struct subprocess_info *sub_info =
                container_of(work, struct subprocess_info, work);
        pid_t pid;
-       int wait = sub_info->wait;
+       enum umh_wait wait = sub_info->wait;
 
        /* 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 || wait == UMH_NO_WAIT)
                pid = kernel_thread(wait_for_helper, sub_info,
                                    CLONE_FS | CLONE_FILES | SIGCHLD);
        else
                pid = kernel_thread(____call_usermodehelper, sub_info,
                                    CLONE_VFORK | SIGCHLD);
 
-       if (wait < 0)
-               return;
-
-       if (pid < 0) {
+       switch (wait) {
+       case UMH_NO_WAIT:
+               break;
+
+       case UMH_WAIT_PROC:
+               if (pid > 0)
+                       break;
                sub_info->retval = pid;
+               /* FALLTHROUGH */
+
+       case UMH_WAIT_EXEC:
                complete(sub_info->complete);
-       } else if (!wait)
-               complete(sub_info->complete);
+       }
 }
 
 /**
@@ -359,7 +364,7 @@ EXPORT_SYMBOL(call_usermodehelper_stdinp
  * (ie. it runs with full root capabilities).
  */
 int call_usermodehelper_exec(struct subprocess_info *sub_info,
-                            int wait)
+                            enum umh_wait wait)
 {
        DECLARE_COMPLETION_ONSTACK(done);
        int retval;
@@ -378,7 +383,7 @@ int call_usermodehelper_exec(struct subp
        sub_info->wait = wait;
 
        queue_work(khelper_wq, &sub_info->work);
-       if (wait < 0) /* task has freed sub_info */
+       if (wait == UMH_NO_WAIT) /* task has freed sub_info */
                return 0;
        wait_for_completion(&done);
        retval = sub_info->retval;
===================================================================
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2318,7 +2318,7 @@ int orderly_poweroff(bool force)
 
        call_usermodehelper_setcleanup(info, argv_cleanup);
 
-       ret = call_usermodehelper_exec(info, -1);
+       ret = call_usermodehelper_exec(info, UMH_NO_WAIT);
 
   out:
        if (ret && force) {
===================================================================
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -208,7 +208,7 @@ int kobject_uevent_env(struct kobject *k
                argv [0] = uevent_helper;
                argv [1] = (char *)subsystem;
                argv [2] = NULL;
-               call_usermodehelper (argv[0], argv, envp, 0);
+               call_usermodehelper (argv[0], argv, envp, UMH_WAIT_EXEC);
        }
 
 exit:
===================================================================
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -125,7 +125,7 @@ static void br_stp_start(struct net_brid
        char *argv[] = { BR_STP_PROG, br->dev->name, "start", NULL };
        char *envp[] = { NULL };
 
-       r = call_usermodehelper(BR_STP_PROG, argv, envp, 1);
+       r = call_usermodehelper(BR_STP_PROG, argv, envp, UMH_WAIT_PROC);
        if (r == 0) {
                br->stp_enabled = BR_USER_STP;
                printk(KERN_INFO "%s: userspace STP started\n", br->dev->name);
===================================================================
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -108,7 +108,8 @@ static int call_sbin_request_key(struct 
        argv[i] = NULL;
 
        /* do it */
-       ret = call_usermodehelper_keys(argv[0], argv, envp, keyring, 1);
+       ret = call_usermodehelper_keys(argv[0], argv, envp, keyring,
+                                      UMH_WAIT_PROC);
 
 error_link:
        key_put(keyring);

-- 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>