diff options
author | Alan Stern <stern@rowland.harvard.edu> | 2007-10-11 16:47:36 -0400 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@suse.de> | 2007-10-12 17:55:35 -0400 |
commit | d1aa3e6aa8edfeb864af7c930523d9e588b28bea (patch) | |
tree | 1ad0e6ead73cccc43dd1c1997aabb25acb4970d2 | |
parent | 58ed7b94d98245fbad54a0af7ea3317ab1dd6876 (diff) |
USB: fix race in autosuspend reschedule
This patch (as1002) fixes a small race which can occur when a driver
expects usbcore to reschedule an autosuspend request. If the request
arrives too late, it won't be rescheduled. The patch adds an extra
argument to autosuspend_check(), indicating that a reschedule is
needed no matter how much time has elapsed.
It also tries to avoid letting asynchronous changes to the value of
jiffies cause a delay to become negative, by caching a local copy of
the current time.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
-rw-r--r-- | drivers/usb/core/driver.c | 32 |
1 files changed, 16 insertions, 16 deletions
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 8c1eac27f2de..c27bc080d84e 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c | |||
@@ -950,11 +950,11 @@ done: | |||
950 | #ifdef CONFIG_USB_SUSPEND | 950 | #ifdef CONFIG_USB_SUSPEND |
951 | 951 | ||
952 | /* Internal routine to check whether we may autosuspend a device. */ | 952 | /* Internal routine to check whether we may autosuspend a device. */ |
953 | static int autosuspend_check(struct usb_device *udev) | 953 | static int autosuspend_check(struct usb_device *udev, int reschedule) |
954 | { | 954 | { |
955 | int i; | 955 | int i; |
956 | struct usb_interface *intf; | 956 | struct usb_interface *intf; |
957 | unsigned long suspend_time; | 957 | unsigned long suspend_time, j; |
958 | 958 | ||
959 | /* For autosuspend, fail fast if anything is in use or autosuspend | 959 | /* For autosuspend, fail fast if anything is in use or autosuspend |
960 | * is disabled. Also fail if any interfaces require remote wakeup | 960 | * is disabled. Also fail if any interfaces require remote wakeup |
@@ -996,20 +996,20 @@ static int autosuspend_check(struct usb_device *udev) | |||
996 | } | 996 | } |
997 | 997 | ||
998 | /* If everything is okay but the device hasn't been idle for long | 998 | /* If everything is okay but the device hasn't been idle for long |
999 | * enough, queue a delayed autosuspend request. | 999 | * enough, queue a delayed autosuspend request. If the device |
1000 | * _has_ been idle for long enough and the reschedule flag is set, | ||
1001 | * likewise queue a delayed (1 second) autosuspend request. | ||
1000 | */ | 1002 | */ |
1001 | if (time_after(suspend_time, jiffies)) { | 1003 | j = jiffies; |
1004 | if (time_before(j, suspend_time)) | ||
1005 | reschedule = 1; | ||
1006 | else | ||
1007 | suspend_time = j + HZ; | ||
1008 | if (reschedule) { | ||
1002 | if (!timer_pending(&udev->autosuspend.timer)) { | 1009 | if (!timer_pending(&udev->autosuspend.timer)) { |
1003 | |||
1004 | /* The value of jiffies may change between the | ||
1005 | * time_after() comparison above and the subtraction | ||
1006 | * below. That's okay; the system behaves sanely | ||
1007 | * when a timer is registered for the present moment | ||
1008 | * or for the past. | ||
1009 | */ | ||
1010 | queue_delayed_work(ksuspend_usb_wq, &udev->autosuspend, | 1010 | queue_delayed_work(ksuspend_usb_wq, &udev->autosuspend, |
1011 | round_jiffies_relative(suspend_time - jiffies)); | 1011 | round_jiffies_relative(suspend_time - j)); |
1012 | } | 1012 | } |
1013 | return -EAGAIN; | 1013 | return -EAGAIN; |
1014 | } | 1014 | } |
1015 | return 0; | 1015 | return 0; |
@@ -1017,7 +1017,7 @@ static int autosuspend_check(struct usb_device *udev) | |||
1017 | 1017 | ||
1018 | #else | 1018 | #else |
1019 | 1019 | ||
1020 | static inline int autosuspend_check(struct usb_device *udev) | 1020 | static inline int autosuspend_check(struct usb_device *udev, int reschedule) |
1021 | { | 1021 | { |
1022 | return 0; | 1022 | return 0; |
1023 | } | 1023 | } |
@@ -1074,7 +1074,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg) | |||
1074 | udev->do_remote_wakeup = device_may_wakeup(&udev->dev); | 1074 | udev->do_remote_wakeup = device_may_wakeup(&udev->dev); |
1075 | 1075 | ||
1076 | if (udev->auto_pm) { | 1076 | if (udev->auto_pm) { |
1077 | status = autosuspend_check(udev); | 1077 | status = autosuspend_check(udev, 0); |
1078 | if (status < 0) | 1078 | if (status < 0) |
1079 | goto done; | 1079 | goto done; |
1080 | } | 1080 | } |
@@ -1100,7 +1100,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg) | |||
1100 | 1100 | ||
1101 | /* Try another autosuspend when the interfaces aren't busy */ | 1101 | /* Try another autosuspend when the interfaces aren't busy */ |
1102 | if (udev->auto_pm) | 1102 | if (udev->auto_pm) |
1103 | autosuspend_check(udev); | 1103 | autosuspend_check(udev, status == -EBUSY); |
1104 | 1104 | ||
1105 | /* If the suspend succeeded then prevent any more URB submissions, | 1105 | /* If the suspend succeeded then prevent any more URB submissions, |
1106 | * flush any outstanding URBs, and propagate the suspend up the tree. | 1106 | * flush any outstanding URBs, and propagate the suspend up the tree. |