* [PATCH net] r8152: use cancel_delayed_work for runtime suspend
@ 2014-10-17 5:55 Hayes Wang
2014-10-17 7:42 ` Bjørn Mork
[not found] ` <1394712342-15778-64-Taiwan-albertk-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
0 siblings, 2 replies; 6+ messages in thread
From: Hayes Wang @ 2014-10-17 5:55 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
It would cause dead lock for runtime suspend, when the workqueue
is running and runtime suspend occurs before the workqueue wakes
up the device. The rtl8152_suspend() waits the workqueue to finish
because of calling cancel_delayed_work_sync(). The workqueue waits
the suspend function to finish for waking up the device because of
calling usb_autopm_get_interface().
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 864159e..7d4e55a 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3200,12 +3200,13 @@ static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
if (netif_running(tp->netdev)) {
clear_bit(WORK_ENABLE, &tp->flags);
usb_kill_urb(tp->intr_urb);
- cancel_delayed_work_sync(&tp->schedule);
tasklet_disable(&tp->tl);
if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
+ cancel_delayed_work(&tp->schedule);
rtl_stop_rx(tp);
rtl_runtime_suspend_enable(tp, true);
} else {
+ cancel_delayed_work_sync(&tp->schedule);
tp->rtl_ops.down(tp);
}
tasklet_enable(&tp->tl);
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net] r8152: use cancel_delayed_work for runtime suspend
2014-10-17 5:55 [PATCH net] r8152: use cancel_delayed_work for runtime suspend Hayes Wang
@ 2014-10-17 7:42 ` Bjørn Mork
2014-10-17 8:55 ` [PATCH net] r8152: return -EBUSY " Hayes Wang
[not found] ` <1394712342-15778-64-Taiwan-albertk-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
1 sibling, 1 reply; 6+ messages in thread
From: Bjørn Mork @ 2014-10-17 7:42 UTC (permalink / raw)
To: Hayes Wang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
Hayes Wang <hayeswang@realtek.com> writes:
> It would cause dead lock for runtime suspend, when the workqueue
> is running and runtime suspend occurs before the workqueue wakes
> up the device. The rtl8152_suspend() waits the workqueue to finish
> because of calling cancel_delayed_work_sync(). The workqueue waits
> the suspend function to finish for waking up the device because of
> calling usb_autopm_get_interface().
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
> drivers/net/usb/r8152.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 864159e..7d4e55a 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -3200,12 +3200,13 @@ static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
> if (netif_running(tp->netdev)) {
> clear_bit(WORK_ENABLE, &tp->flags);
> usb_kill_urb(tp->intr_urb);
> - cancel_delayed_work_sync(&tp->schedule);
> tasklet_disable(&tp->tl);
> if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
> + cancel_delayed_work(&tp->schedule);
> rtl_stop_rx(tp);
> rtl_runtime_suspend_enable(tp, true);
> } else {
> + cancel_delayed_work_sync(&tp->schedule);
> tp->rtl_ops.down(tp);
> }
> tasklet_enable(&tp->tl);
This looks strange to me. The delayed work will cause an immediate
resume due to the usb_autopm_get_interface() it starts with. Wouldn't
it be better to just prevent runtime suspending by returning -EBUSY if
there is any delayed work scheduled?
Bjørn
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net] r8152: return -EBUSY for runtime suspend
2014-10-17 7:42 ` Bjørn Mork
@ 2014-10-17 8:55 ` Hayes Wang
2014-10-18 3:46 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Hayes Wang @ 2014-10-17 8:55 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
Remove calling cancel_delayed_work_sync() for runtime suspend,
because it would cause dead lock. Instead, return -EBUSY to
avoid the device enters suspending if the net is running and
the delayed work is pending or running. The delayed work would
try to wake up the device later, so the suspending is not
necessary.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 864159e..e3d84c3 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3189,31 +3189,39 @@ static void r8153_init(struct r8152 *tp)
static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
{
struct r8152 *tp = usb_get_intfdata(intf);
+ struct net_device *netdev = tp->netdev;
+ int ret = 0;
mutex_lock(&tp->control);
- if (PMSG_IS_AUTO(message))
+ if (PMSG_IS_AUTO(message)) {
+ if (netif_running(netdev) && work_busy(&tp->schedule.work)) {
+ ret = -EBUSY;
+ goto out1;
+ }
+
set_bit(SELECTIVE_SUSPEND, &tp->flags);
- else
- netif_device_detach(tp->netdev);
+ } else {
+ netif_device_detach(netdev);
+ }
- if (netif_running(tp->netdev)) {
+ if (netif_running(netdev)) {
clear_bit(WORK_ENABLE, &tp->flags);
usb_kill_urb(tp->intr_urb);
- cancel_delayed_work_sync(&tp->schedule);
tasklet_disable(&tp->tl);
if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
rtl_stop_rx(tp);
rtl_runtime_suspend_enable(tp, true);
} else {
+ cancel_delayed_work_sync(&tp->schedule);
tp->rtl_ops.down(tp);
}
tasklet_enable(&tp->tl);
}
-
+out1:
mutex_unlock(&tp->control);
- return 0;
+ return ret;
}
static int rtl8152_resume(struct usb_interface *intf)
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net] r8152: return -EBUSY for runtime suspend
2014-10-17 8:55 ` [PATCH net] r8152: return -EBUSY " Hayes Wang
@ 2014-10-18 3:46 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2014-10-18 3:46 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
From: Hayes Wang <hayeswang@realtek.com>
Date: Fri, 17 Oct 2014 16:55:08 +0800
> Remove calling cancel_delayed_work_sync() for runtime suspend,
> because it would cause dead lock. Instead, return -EBUSY to
> avoid the device enters suspending if the net is running and
> the delayed work is pending or running. The delayed work would
> try to wake up the device later, so the suspending is not
> necessary.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <1394712342-15778-64-Taiwan-albertk-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>]
end of thread, other threads:[~2014-10-20 2:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-17 5:55 [PATCH net] r8152: use cancel_delayed_work for runtime suspend Hayes Wang
2014-10-17 7:42 ` Bjørn Mork
2014-10-17 8:55 ` [PATCH net] r8152: return -EBUSY " Hayes Wang
2014-10-18 3:46 ` David Miller
[not found] ` <1394712342-15778-64-Taiwan-albertk-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
2014-10-18 19:48 ` [PATCH net] r8152: use cancel_delayed_work " Oliver Neukum
[not found] ` <1413661699.19391.2.camel-AfvqVibwNMkMNNZnWhT/Jw@public.gmane.org>
2014-10-20 2:19 ` Hayes Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).