diff options
| author | Tejun Heo <tj@kernel.org> | 2008-09-24 17:22:23 -0400 |
|---|---|---|
| committer | Eric Van Hensbergen <ericvh@ericvh-desktop.austin.ibm.com> | 2008-09-24 17:22:23 -0400 |
| commit | 7dc5d24be06a5ed874af035d52a083a7b61ef1bd (patch) | |
| tree | 74f8e59f6a32ffc099dca3eb1eed9d6e6b58b616 | |
| parent | 72029fe85d8d060b3f966f2dbc36b3c75b5a6532 (diff) | |
9p-trans_fd: fix trans_fd::p9_conn_destroy()
p9_conn_destroy() first kills all current requests by calling
p9_conn_cancel(), then waits for the request list to be cleared by
waiting on p9_conn->equeue. After that, polling is stopped and the
trans is destroyed. This sequence has a few problems.
* Read and write works were never cancelled and the p9_conn can be
destroyed while the works are running as r/w works remove requests
from the list and dereference the p9_conn from them.
* The list emptiness wait using p9_conn->equeue wouldn't trigger
because p9_conn_cancel() always clears all the lists and the only
way the wait can be triggered is to have another task to issue a
request between the slim window between p9_conn_cancel() and the
wait, which isn't safe under the current implementation with or
without the wait.
This patch fixes the problem by first stopping poll, which can
schedule r/w works, first and cancle r/w works which guarantees that
r/w works are not and will not run from that point and then calling
p9_conn_cancel() and do the rest of destruction.
Signed-off-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Eric Van Hensbergen <ericvh@gmail.com>
| -rw-r--r-- | net/9p/trans_fd.c | 24 |
1 files changed, 5 insertions, 19 deletions
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c index 6a32ffdb9429..ee0d151da31a 100644 --- a/net/9p/trans_fd.c +++ b/net/9p/trans_fd.c | |||
| @@ -151,7 +151,6 @@ struct p9_mux_poll_task { | |||
| 151 | * @trans: reference to transport instance for this connection | 151 | * @trans: reference to transport instance for this connection |
| 152 | * @tagpool: id accounting for transactions | 152 | * @tagpool: id accounting for transactions |
| 153 | * @err: error state | 153 | * @err: error state |
| 154 | * @equeue: event wait_q (?) | ||
| 155 | * @req_list: accounting for requests which have been sent | 154 | * @req_list: accounting for requests which have been sent |
| 156 | * @unsent_req_list: accounting for requests that haven't been sent | 155 | * @unsent_req_list: accounting for requests that haven't been sent |
| 157 | * @rcall: current response &p9_fcall structure | 156 | * @rcall: current response &p9_fcall structure |
| @@ -178,7 +177,6 @@ struct p9_conn { | |||
| 178 | struct p9_trans *trans; | 177 | struct p9_trans *trans; |
| 179 | struct p9_idpool *tagpool; | 178 | struct p9_idpool *tagpool; |
| 180 | int err; | 179 | int err; |
| 181 | wait_queue_head_t equeue; | ||
| 182 | struct list_head req_list; | 180 | struct list_head req_list; |
| 183 | struct list_head unsent_req_list; | 181 | struct list_head unsent_req_list; |
| 184 | struct p9_fcall *rcall; | 182 | struct p9_fcall *rcall; |
| @@ -430,7 +428,6 @@ static struct p9_conn *p9_conn_create(struct p9_trans *trans) | |||
| 430 | } | 428 | } |
| 431 | 429 | ||
| 432 | m->err = 0; | 430 | m->err = 0; |
| 433 | init_waitqueue_head(&m->equeue); | ||
| 434 | INIT_LIST_HEAD(&m->req_list); | 431 | INIT_LIST_HEAD(&m->req_list); |
| 435 | INIT_LIST_HEAD(&m->unsent_req_list); | 432 | INIT_LIST_HEAD(&m->unsent_req_list); |
| 436 | m->rcall = NULL; | 433 | m->rcall = NULL; |
| @@ -483,18 +480,13 @@ static void p9_conn_destroy(struct p9_conn *m) | |||
| 483 | { | 480 | { |
| 484 | P9_DPRINTK(P9_DEBUG_MUX, "mux %p prev %p next %p\n", m, | 481 | P9_DPRINTK(P9_DEBUG_MUX, "mux %p prev %p next %p\n", m, |
| 485 | m->mux_list.prev, m->mux_list.next); | 482 | m->mux_list.prev, m->mux_list.next); |
| 486 | p9_conn_cancel(m, -ECONNRESET); | ||
| 487 | |||
| 488 | if (!list_empty(&m->req_list)) { | ||
| 489 | /* wait until all processes waiting on this session exit */ | ||
| 490 | P9_DPRINTK(P9_DEBUG_MUX, | ||
| 491 | "mux %p waiting for empty request queue\n", m); | ||
| 492 | wait_event_timeout(m->equeue, (list_empty(&m->req_list)), 5000); | ||
| 493 | P9_DPRINTK(P9_DEBUG_MUX, "mux %p request queue empty: %d\n", m, | ||
| 494 | list_empty(&m->req_list)); | ||
| 495 | } | ||
| 496 | 483 | ||
| 497 | p9_mux_poll_stop(m); | 484 | p9_mux_poll_stop(m); |
| 485 | cancel_work_sync(&m->rq); | ||
| 486 | cancel_work_sync(&m->wq); | ||
| 487 | |||
| 488 | p9_conn_cancel(m, -ECONNRESET); | ||
| 489 | |||
| 498 | m->trans = NULL; | 490 | m->trans = NULL; |
| 499 | p9_idpool_destroy(m->tagpool); | 491 | p9_idpool_destroy(m->tagpool); |
| 500 | kfree(m); | 492 | kfree(m); |
| @@ -840,8 +832,6 @@ static void p9_read_work(struct work_struct *work) | |||
| 840 | (*req->cb) (req, req->cba); | 832 | (*req->cb) (req, req->cba); |
| 841 | else | 833 | else |
| 842 | kfree(req->rcall); | 834 | kfree(req->rcall); |
| 843 | |||
| 844 | wake_up(&m->equeue); | ||
| 845 | } | 835 | } |
| 846 | } else { | 836 | } else { |
| 847 | if (err >= 0 && rcall->id != P9_RFLUSH) | 837 | if (err >= 0 && rcall->id != P9_RFLUSH) |
| @@ -984,8 +974,6 @@ static void p9_mux_flush_cb(struct p9_req *freq, void *a) | |||
| 984 | (*req->cb) (req, req->cba); | 974 | (*req->cb) (req, req->cba); |
| 985 | else | 975 | else |
| 986 | kfree(req->rcall); | 976 | kfree(req->rcall); |
| 987 | |||
| 988 | wake_up(&m->equeue); | ||
| 989 | } | 977 | } |
| 990 | 978 | ||
| 991 | kfree(freq->tcall); | 979 | kfree(freq->tcall); |
| @@ -1191,8 +1179,6 @@ void p9_conn_cancel(struct p9_conn *m, int err) | |||
| 1191 | else | 1179 | else |
| 1192 | kfree(req->rcall); | 1180 | kfree(req->rcall); |
| 1193 | } | 1181 | } |
| 1194 | |||
| 1195 | wake_up(&m->equeue); | ||
| 1196 | } | 1182 | } |
| 1197 | 1183 | ||
| 1198 | /** | 1184 | /** |
