Messages in this thread | | | Date | Mon, 7 Jan 2008 00:53:56 +0300 | From | Cyrill Gorcunov <> | Subject | Re: 2.6.24-rc6: possible recursive locking detected |
| |
[Cyrill Gorcunov - Mon, Jan 07, 2008 at 12:44:42AM +0300] | [Davide Libenzi - Sat, Jan 05, 2008 at 01:35:25PM -0800] | | On Sat, 5 Jan 2008, Peter Zijlstra wrote: | | | [...snip...] | | I remember I talked with Arjan about this time ago. Basically, since 1) | | you can drop an epoll fd inside another epoll fd 2) callback-based wakeups | | are used, you can see a wake_up() from inside another wake_up(), but they | | will never refer to the same lock instance. | | Think about: | | | | dfd = socket(...); | | efd1 = epoll_create(); | | efd2 = epoll_create(); | | epoll_ctl(efd1, EPOLL_CTL_ADD, dfd, ...); | | epoll_ctl(efd2, EPOLL_CTL_ADD, efd1, ...); | | | | When a packet arrives to the device underneath "dfd", the net code will | | issue a wake_up() on its poll wake list. Epoll (efd1) has installed a | | callback wakeup entry on that queue, and the wake_up() performed by the | | "dfd" net code will end up in ep_poll_callback(). At this point epoll | | (efd1) notices that it may have some event ready, so it needs to wake up | | the waiters on its poll wait list (efd2). So it calls ep_poll_safewake() | | that ends up in another wake_up(), after having checked about the | | recursion constraints. That are, no more than EP_MAX_POLLWAKE_NESTS, to | | avoid stack blasting. Never hit the same queue, to avoid loops like: | | | | epoll_ctl(efd2, EPOLL_CTL_ADD, efd1, ...); | | epoll_ctl(efd3, EPOLL_CTL_ADD, efd2, ...); | | epoll_ctl(efd4, EPOLL_CTL_ADD, efd3, ...); | | epoll_ctl(efd1, EPOLL_CTL_ADD, efd4, ...); | | | | The code "if (tncur->wq == wq || ..." prevents re-entering the same | | queue/lock. | | I don't know how the lockdep code works, so I can't say about | | wake_up_nested(). Although I have a feeling is not enough in this case. | | A solution may be to move the call to ep_poll_safewake() (that'd become a | | simple wake_up()) inside a tasklet or whatever is today trendy for delayed | | work. But his kinda scares me to be honest, since epoll has already a | | bunch of places where it could be asynchronously hit (plus performance | | regression will need to be verified). | | | | | | | | - Davide | | | | | | it's quite possible that i'm wrong but just interested... | why in ep_poll_safewake() the assignment | | struct list_head *lsthead = &psw->wake_task_list; | | is not protected by spinlock? | | - Cyrill -
it was a completely stupid question... please drop ;)
- Cyrill -
| |