* [PATCH]: Fix deadlock in USBIP driver (staging), linux-2.6.34-rc5
@ 2010-04-22 16:54 Eric Lescouet
2010-04-22 17:02 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Eric Lescouet @ 2010-04-22 16:54 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, Greg Kroah-Hartman
Cc: usbip-devel@lists.sourceforge.net
Hi,
When detaching a port from the client side (usbip --detach 0),
the event thread, on the server side, is going to deadlock.
The "eh" server thread is getting USBIP_EH_RESET event and calls:
-> stub_device_reset() -> usb_reset_device()
the USB framework is then calling back _in the same "eh" thread_ :
-> stub_disconnect() -> usbip_stop_eh() -> wait_for_completion()
the "eh" thread is being asleep forever, waiting for its own completion.
The patch checks if "eh" is the current thread, in usbip_stop_eh().
Please Cc me in reply, I'm not in the list.
b.r.
------------------
diff -Nur linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c
linux-2.6.34-rc5.new/drivers/staging/usbip/usbip_event.c
--- linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c 2010-04-20
01:29:56.000000000 +0200
+++ linux-2.6.34-rc5.new/drivers/staging/usbip/usbip_event.c
2010-04-22 17:07:36.249588273 +0200
@@ -116,6 +116,13 @@
void usbip_stop_eh(struct usbip_device *ud)
{
struct usbip_task *eh = &ud->eh;
+ int i_am_eh;
+
+ lock_kernel();
+ i_am_eh = (eh->thread == current);
+ unlock_kernel();
+ if (i_am_eh)
+ return; /* do not wait for myself */
wait_for_completion(&eh->thread_done);
usbip_dbg_eh("usbip_eh has finished\n");
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH]: Fix deadlock in USBIP driver (staging), linux-2.6.34-rc5
2010-04-22 16:54 [PATCH]: Fix deadlock in USBIP driver (staging), linux-2.6.34-rc5 Eric Lescouet
@ 2010-04-22 17:02 ` Greg KH
2010-04-22 18:12 ` Eric Lescouet
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2010-04-22 17:02 UTC (permalink / raw)
To: Eric Lescouet
Cc: linux-kernel@vger.kernel.org, usbip-devel@lists.sourceforge.net
On Thu, Apr 22, 2010 at 06:54:02PM +0200, Eric Lescouet wrote:
> Hi,
> When detaching a port from the client side (usbip --detach 0),
> the event thread, on the server side, is going to deadlock.
> The "eh" server thread is getting USBIP_EH_RESET event and calls:
> -> stub_device_reset() -> usb_reset_device()
> the USB framework is then calling back _in the same "eh" thread_ :
> -> stub_disconnect() -> usbip_stop_eh() -> wait_for_completion()
> the "eh" thread is being asleep forever, waiting for its own completion.
> The patch checks if "eh" is the current thread, in usbip_stop_eh().
Ok, but does the lock_kernel() call you just made actually prevent this
from happening? Isn't there some other lock you can use instead?
> Please Cc me in reply, I'm not in the list.
Also, to have patches that we can apply, we need a "Signed-off-by:"
line. See the file, Documentation/SubmittingPatches for what this
means.
>
> b.r.
>
> ------------------
>
> diff -Nur linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c
> linux-2.6.34-rc5.new/drivers/staging/usbip/usbip_event.c
> --- linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c
> 2010-04-20 01:29:56.000000000 +0200
Your email client wrapped the patch and made it unappliable :(
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH]: Fix deadlock in USBIP driver (staging), linux-2.6.34-rc5
2010-04-22 17:02 ` Greg KH
@ 2010-04-22 18:12 ` Eric Lescouet
2010-04-22 18:51 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Eric Lescouet @ 2010-04-22 18:12 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel@vger.kernel.org, usbip-devel@lists.sourceforge.net
Greg KH wrote:
[...]
> Ok, but does the lock_kernel() call you just made actually prevent this
> from happening? Isn't there some other lock you can use instead?
>
Right. lock_kernel() is not needed (was copy/paste).
The deadlock is avoided by not calling wait_for_completion()
when the code is executed by the "eh" thread itself.
> Also, to have patches that we can apply, we need a "Signed-off-by:"
> line. See the file, Documentation/SubmittingPatches for what this
> means.
Understood.
> Your email client wrapped the patch and made it unappliable :(
>
> thanks,
>
> greg k-h
Sorry about that. Please find below an hopefully better one.
Thanks,
Eric.
Signed-off-by: Eric Lescouet <eric@lescouet.org>
-------------
diff -Nur linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c linux-2.6.34-rc5.new/drivers/staging/usbip/usbip_event.c
--- linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c 2010-04-20 01:29:56.000000000 +0200
+++ linux-2.6.34-rc5.new/drivers/staging/usbip/usbip_event.c 2010-04-22 19:19:52.997889126 +0200
@@ -117,6 +117,9 @@
{
struct usbip_task *eh = &ud->eh;
+ if (eh->thread == current)
+ return; /* do not wait for myself */
+
wait_for_completion(&eh->thread_done);
usbip_dbg_eh("usbip_eh has finished\n");
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH]: Fix deadlock in USBIP driver (staging), linux-2.6.34-rc5
2010-04-22 18:12 ` Eric Lescouet
@ 2010-04-22 18:51 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2010-04-22 18:51 UTC (permalink / raw)
To: Eric Lescouet
Cc: linux-kernel@vger.kernel.org, usbip-devel@lists.sourceforge.net
On Thu, Apr 22, 2010 at 08:12:05PM +0200, Eric Lescouet wrote:
> Greg KH wrote:
> [...]
> >Ok, but does the lock_kernel() call you just made actually prevent this
> >from happening? Isn't there some other lock you can use instead?
> >
>
> Right. lock_kernel() is not needed (was copy/paste).
> The deadlock is avoided by not calling wait_for_completion()
> when the code is executed by the "eh" thread itself.
>
> >Also, to have patches that we can apply, we need a "Signed-off-by:"
> >line. See the file, Documentation/SubmittingPatches for what this
> >means.
>
> Understood.
>
> >Your email client wrapped the patch and made it unappliable :(
> >
> >thanks,
> >
> >greg k-h
>
> Sorry about that. Please find below an hopefully better one.
> Thanks,
> Eric.
>
>
> Signed-off-by: Eric Lescouet <eric@lescouet.org>
> -------------
>
> diff -Nur linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c
> linux-2.6.34-rc5.new/drivers/staging/usbip/usbip_event.c
> --- linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c
> 2010-04-20 01:29:56.000000000 +0200
> +++ linux-2.6.34-rc5.new/drivers/staging/usbip/usbip_event.c
> 2010-04-22 19:19:52.997889126 +0200
> @@ -117,6 +117,9 @@
> {
> struct usbip_task *eh = &ud->eh;
> + if (eh->thread == current)
> + return; /* do not wait for myself */
> +
> wait_for_completion(&eh->thread_done);
> usbip_dbg_eh("usbip_eh has finished\n");
Odd, the patch has all leading tabs and spaces stripped out of it, is
linewrapped, and the tabs are converted to spaces :(
Care to try again, third time's a charm...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH]: Fix deadlock in USBIP driver (staging), linux-2.6.34-rc5
@ 2010-04-22 22:10 Eric Lescouet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Lescouet @ 2010-04-22 22:10 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, usbip-devel
> Odd, the patch has all leading tabs and spaces stripped out of it, is
> linewrapped, and the tabs are converted to spaces :(
>
> Care to try again, third time's a charm...
OK. It looked better on my box and on the list as well ...
... but let's try another client.
Eric.
Signed-off-by: Eric Lescouet <eric@lescouet.org>
-------------
diff -Nur linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c linux-2.6.34-rc5.new/drivers/staging/usbip/usbip_event.c
--- linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c 2010-04-20 01:29:56.000000000 +0200
+++ linux-2.6.34-rc5.new/drivers/staging/usbip/usbip_event.c 2010-04-22 19:19:52.997889126 +0200
@@ -117,6 +117,9 @@
{
struct usbip_task *eh = &ud->eh;
+ if (eh->thread == current)
+ return; /* do not wait for myself */
+
wait_for_completion(&eh->thread_done);
usbip_dbg_eh("usbip_eh has finished\n");
}
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH]: Fix deadlock in USBIP driver (staging), linux-2.6.34-rc5
@ 2010-04-24 0:55 Eric Lescouet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Lescouet @ 2010-04-24 0:55 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel
Resent with hopefully everything ...
When detaching a port from the client side (usbip --detach 0),
the event thread, on the server side, is going to deadlock.
The "eh" server thread is getting USBIP_EH_RESET event and calls:
-> stub_device_reset() -> usb_reset_device()
the USB framework is then calling back _in the same "eh" thread_ :
-> stub_disconnect() -> usbip_stop_eh() -> wait_for_completion()
the "eh" thread is being asleep forever, waiting for its own completion.
This patch checks if "eh" is the current thread, in usbip_stop_eh().
Signed-off-by: Eric Lescouet <eric@lescouet.org>
---
diff -Nur linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c linux-2.6.34-rc5.new/drivers/staging/usbip/usbip_event.c
--- linux-2.6.34-rc5/drivers/staging/usbip/usbip_event.c 2010-04-20 01:29:56.000000000 +0200
+++ linux-2.6.34-rc5.new/drivers/staging/usbip/usbip_event.c 2010-04-22 19:19:52.997889126 +0200
@@ -117,6 +117,9 @@
{
struct usbip_task *eh = &ud->eh;
+ if (eh->thread == current)
+ return; /* do not wait for myself */
+
wait_for_completion(&eh->thread_done);
usbip_dbg_eh("usbip_eh has finished\n");
}
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-04-24 0:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-22 16:54 [PATCH]: Fix deadlock in USBIP driver (staging), linux-2.6.34-rc5 Eric Lescouet
2010-04-22 17:02 ` Greg KH
2010-04-22 18:12 ` Eric Lescouet
2010-04-22 18:51 ` Greg KH
-- strict thread matches above, loose matches on Subject: below --
2010-04-22 22:10 Eric Lescouet
2010-04-24 0:55 Eric Lescouet
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).