Revert "PM / Sleep: Mitigate race between the freezer and request_firmware()"

This reverts commit 247bc03742545fec2f79939a3b9f738392a0f7b4.

This patch caused a deadlock between khubd and the suspend process.
This was reliably reproducible when we did a suspend immediately after
a resume, and the khubd kernel thread was busy probing USB devices.

The sequence was that khubd would be running and probing USB devices
when we try to suspend and the freezer would activate, and eventually
khubd would call request_firmware() while holding a device lock, and
request_firmware would call usermodehelper_read_trylock().
Because the UMH is in state UMH_FREEZING, usermode_helper_read_trylock
would end up calling try_to_freeze() and would then be frozen while
holding a device lock.  After the freezer finished, it would then try
to suspend each device and would deadlock on the USB hub device that
was being held by khubd.

Reverting this merely results in a WARN_ON from request_firmware() and
no deadlock, and upon the subsequent resume, khubd will successfully
get the firmware and the devices work correctly.

Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
BUG=chrome-os-partner:17270
TEST=(needs an autotest test) currently manual:
use the set_short_powerd_timeouts script to all the device to
idle-suspend quickly, wait for an idle suspend with the lid open
(about 30  seconds). Then hit the trackpad and quickly close the lid.
The device should resume and then suspend again within a few seconds.

Change-Id: Idfc5d40f0f1cc88ca335502454bf675f152df477
Reviewed-on: https://gerrit.chromium.org/gerrit/42190
Reviewed-by: Sameer Nanda <snanda@chromium.org>
Commit-Queue: Sonny Rao <sonnyrao@chromium.org>
Tested-by: Sonny Rao <sonnyrao@chromium.org>

diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index dd99c32..b087377 100644 (file)
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -110,27 +110,10 @@ call_usermodehelper(char *path, char **argv, char **envp, int wait)
 
 extern struct ctl_table usermodehelper_table[];
 
-enum umh_disable_depth {
-       UMH_ENABLED = 0,
-       UMH_FREEZING,
-       UMH_DISABLED,
-};
-
 extern void usermodehelper_init(void);
 
-extern int __usermodehelper_disable(enum umh_disable_depth depth);
-extern void __usermodehelper_set_disable_depth(enum umh_disable_depth depth);
-
-static inline int usermodehelper_disable(void)
-{
-       return __usermodehelper_disable(UMH_DISABLED);
-}
-
-static inline void usermodehelper_enable(void)
-{
-       __usermodehelper_set_disable_depth(UMH_ENABLED);
-}
-
+extern int usermodehelper_disable(void);
+extern void usermodehelper_enable(void);
 extern int usermodehelper_read_trylock(void);
 extern long usermodehelper_read_lock_wait(long timeout);
 extern void usermodehelper_read_unlock(void);

diff --git a/kernel/kmod.c b/kernel/kmod.c
index 05698a7..da7fcca 100644 (file)
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -322,7 +322,7 @@ static void __call_usermodehelper(struct work_struct *work)
  * land has been frozen during a system-wide hibernation or suspend operation).
  * Should always be manipulated under umhelper_sem acquired for write.
  */
-static enum umh_disable_depth usermodehelper_disabled = UMH_DISABLED;
+static int usermodehelper_disabled = 1;
 
 /* Number of helpers running */
 static atomic_t running_helpers = ATOMIC_INIT(0);
@@ -347,30 +347,13 @@ static DECLARE_WAIT_QUEUE_HEAD(usermodehelper_disabled_waitq);
 
 int usermodehelper_read_trylock(void)
 {
-       DEFINE_WAIT(wait);
        int ret = 0;
 
        down_read(&umhelper_sem);
-       for (;;) {
-               prepare_to_wait(&usermodehelper_disabled_waitq, &wait,
-                               TASK_INTERRUPTIBLE);
-               if (!usermodehelper_disabled)
-                       break;
-
-               if (usermodehelper_disabled == UMH_DISABLED)
-                       ret = -EAGAIN;
-
+       if (usermodehelper_disabled) {
                up_read(&umhelper_sem);
-
-               if (ret)
-                       break;
-
-               schedule();
-               try_to_freeze();
-
-               down_read(&umhelper_sem);
+               ret = -EAGAIN;
        }
-       finish_wait(&usermodehelper_disabled_waitq, &wait);
        return ret;
 }
 EXPORT_SYMBOL_GPL(usermodehelper_read_trylock);
@@ -409,35 +392,25 @@ void usermodehelper_read_unlock(void)
 EXPORT_SYMBOL_GPL(usermodehelper_read_unlock);
 
 /**
- * __usermodehelper_set_disable_depth - Modify usermodehelper_disabled.
- * depth: New value to assign to usermodehelper_disabled.
- *
- * Change the value of usermodehelper_disabled (under umhelper_sem locked for
- * writing) and wakeup tasks waiting for it to change.
+ * usermodehelper_enable - allow new helpers to be started again
  */
-void __usermodehelper_set_disable_depth(enum umh_disable_depth depth)
+void usermodehelper_enable(void)
 {
        down_write(&umhelper_sem);
-       usermodehelper_disabled = depth;
+       usermodehelper_disabled = 0;
        wake_up(&usermodehelper_disabled_waitq);
        up_write(&umhelper_sem);
 }
 
 /**
- * __usermodehelper_disable - Prevent new helpers from being started.
- * @depth: New value to assign to usermodehelper_disabled.
- *
- * Set usermodehelper_disabled to @depth and wait for running helpers to exit.
+ * usermodehelper_disable - prevent new helpers from being started
  */
-int __usermodehelper_disable(enum umh_disable_depth depth)
+int usermodehelper_disable(void)
 {
        long retval;
 
-       if (!depth)
-               return -EINVAL;
-
        down_write(&umhelper_sem);
-       usermodehelper_disabled = depth;
+       usermodehelper_disabled = 1;
        up_write(&umhelper_sem);
 
        /*
@@ -452,7 +425,7 @@ int __usermodehelper_disable(enum umh_disable_depth depth)
        if (retval)
                return 0;
 
-       __usermodehelper_set_disable_depth(UMH_ENABLED);
+       usermodehelper_enable();
        return -EAGAIN;
 }
 

diff --git a/kernel/power/process.c b/kernel/power/process.c
index 19db29f..56eaac7 100644 (file)
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -123,7 +123,7 @@ int freeze_processes(void)
 {
        int error;
 
-       error = __usermodehelper_disable(UMH_FREEZING);
+       error = usermodehelper_disable();
        if (error)
                return error;
 
@@ -135,7 +135,6 @@ int freeze_processes(void)
        error = try_to_freeze_tasks(true);
        if (!error) {
                printk("done.");
-               __usermodehelper_set_disable_depth(UMH_DISABLED);
                oom_killer_disable();
        }
        printk("\n");