From: robert.jarzmik@free.fr (Robert Jarzmik)
To: linux-arm-kernel@lists.infradead.org
Subject: [BUG] pxa27x_udc: possible recursive locking detected in pxa_ep_queue
Date: Sun, 06 Dec 2009 19:34:53 +0100 [thread overview]
Message-ID: <87638k9cj6.fsf@free.fr> (raw)
In-Reply-To: <20091205115754.7e1dc0fd.ospite@studenti.unina.it> (Antonio Ospite's message of "Sat\, 5 Dec 2009 11\:57\:54 +0100")
Antonio Ospite <ospite@studenti.unina.it> writes:
> Hi,
>
> I've run into this recently, I get it with 2.6.32 (plus some code for
> the EZX platform) especially using ROOT_NFS over usblan. It looks like
> I can also trigger it regularly by connecting and disconnecting usb
> cable repeatedly while the kernel on the pxa system is loading
> (in a _non_ ROOT_NFS scenario).
Your discovery is very ... unfortunate for me.
What you discovered is a real locking issue in pxa27x_udc, which can be
outlined as :
1) an irq comes in for endpoint 1 (OUT endpoint)
2) irq handler kick in
handle_ep()
3) the packet is smaller than the endpoint fifo
3a) it gets read fully
3b) it's a usb short packet
3c) the transfer is completed
req_done() is called
4) req_done() calls gadget layer
req->req.complete()
5) gadget layer complete() function pushes another request to pxa27x_udc
(notice we're still in the irq handler)
pxa_ep_queue()
(notice we take the ep->lock)
6) pxa27x_udc calls handle_ep()
7) same as (3)
8) same as (4)
9) same as (5)
=> here, pxa_ep_queue() tries to take the ep->lock twice !!!
=> this is the deadlock
Summary is :
irq_handler
\
-> gadget.complete()
\
-> pxa27x_udc.pxa_ep_queue() : implies ep->lock is taken
\
-> gadget.complete()
\
-> pxa27x_udc.pxa_ep_queue() : implies ep->lock is attempted
==> *deadlock*
The point here an architectural one : can the gadget layer, in its completion
method, call endpoint queuing methods ?
If so, when nuke() is called, gadget_complete() is always called, which could
call request queuing, etc ..., which will become an infinite loop.
I may modify the locking model of pxa27x_udc : whenether I call the gadget
complete() method, I relax the ep->lock, and take it just after. That makes me a
bit nervous, but I'll do it if this is the thing to do.
David, could you give me the point of view of the gadget architecture please ?
Cheers.
--
Robert
next prev parent reply other threads:[~2009-12-06 18:34 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-05 10:57 [BUG] pxa27x_udc: possible recursive locking detected in pxa_ep_queue Antonio Ospite
2009-12-06 18:34 ` Robert Jarzmik [this message]
2009-12-06 20:01 ` Alan Stern
2009-12-06 20:23 ` David Brownell
2009-12-10 17:58 ` Robert Jarzmik
2009-12-10 21:01 ` David Brownell
2009-12-06 20:13 ` David Brownell
2009-12-10 17:49 ` Robert Jarzmik
2009-12-12 14:28 ` Robert Jarzmik
2009-12-12 16:31 ` Antonio Ospite
2009-12-20 18:36 ` Robert Jarzmik
2009-12-22 23:53 ` Antonio Ospite
2009-12-28 20:23 ` Robert Jarzmik
2009-12-28 23:03 ` Antonio Ospite
2010-01-17 12:41 ` Antonio Ospite
2010-01-17 19:33 ` Robert Jarzmik
2010-03-30 21:26 ` Michael Trimarchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87638k9cj6.fsf@free.fr \
--to=robert.jarzmik@free.fr \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.