diff options
author | Bart Van Assche <bart.vanassche@sandisk.com> | 2016-04-07 18:55:04 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2016-04-07 21:16:20 -0400 |
commit | 3c9688876ace9ca4cd8630e5fbba8bb28235990a (patch) | |
tree | 6d72a0ca9d2b72522f9dd54dbab510dd6fbf7c63 | |
parent | 93061f390f107c37bad7e3bf9eb07bda58a4a99f (diff) |
Revert "ib_srpt: Convert to percpu_ida tag allocation"
This reverts commit 0fd10721fe3664f7549e74af9d28a509c9a68719.
That patch causes the ib_srpt driver to crash as soon as the first SCSI
command is received:
kernel BUG at drivers/infiniband/ulp/srpt/ib_srpt.c:1439!
invalid opcode: 0000 [#1] SMP
Workqueue: target_completion target_complete_ok_work [target_core_mod]
RIP: srpt_queue_response+0x437/0x4a0 [ib_srpt]
Call Trace:
srpt_queue_data_in+0x9/0x10 [ib_srpt]
target_complete_ok_work+0x152/0x2b0 [target_core_mod]
process_one_work+0x197/0x480
worker_thread+0x49/0x490
kthread+0xea/0x100
ret_from_fork+0x22/0x40
Aside from the crash, the shortcomings of that patch are as follows:
- It makes the ib_srpt driver use I/O contexts allocated by
transport_alloc_session_tags() but it does not initialize these I/O
contexts properly. All the initializations performed by
srpt_alloc_ioctx() are skipped.
- It swaps the order of the send ioctx allocation and the transition to
RTR mode which is wrong.
- The amount of memory that is needed for I/O contexts is doubled.
- srpt_rdma_ch.free_list is no longer used but is not removed.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Nicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r-- | drivers/infiniband/ulp/srpt/ib_srpt.c | 55 | ||||
-rw-r--r-- | drivers/infiniband/ulp/srpt/ib_srpt.h | 2 |
2 files changed, 40 insertions, 17 deletions
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c index 0bd3cb2f3c67..8b42401d4795 100644 --- a/drivers/infiniband/ulp/srpt/ib_srpt.c +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c | |||
@@ -1264,26 +1264,40 @@ free_mem: | |||
1264 | */ | 1264 | */ |
1265 | static struct srpt_send_ioctx *srpt_get_send_ioctx(struct srpt_rdma_ch *ch) | 1265 | static struct srpt_send_ioctx *srpt_get_send_ioctx(struct srpt_rdma_ch *ch) |
1266 | { | 1266 | { |
1267 | struct se_session *se_sess; | ||
1268 | struct srpt_send_ioctx *ioctx; | 1267 | struct srpt_send_ioctx *ioctx; |
1269 | int tag; | 1268 | unsigned long flags; |
1270 | 1269 | ||
1271 | BUG_ON(!ch); | 1270 | BUG_ON(!ch); |
1272 | se_sess = ch->sess; | ||
1273 | 1271 | ||
1274 | tag = percpu_ida_alloc(&se_sess->sess_tag_pool, TASK_RUNNING); | 1272 | ioctx = NULL; |
1275 | if (tag < 0) { | 1273 | spin_lock_irqsave(&ch->spinlock, flags); |
1276 | pr_err("Unable to obtain tag for srpt_send_ioctx\n"); | 1274 | if (!list_empty(&ch->free_list)) { |
1277 | return NULL; | 1275 | ioctx = list_first_entry(&ch->free_list, |
1276 | struct srpt_send_ioctx, free_list); | ||
1277 | list_del(&ioctx->free_list); | ||
1278 | } | 1278 | } |
1279 | ioctx = &((struct srpt_send_ioctx *)se_sess->sess_cmd_map)[tag]; | 1279 | spin_unlock_irqrestore(&ch->spinlock, flags); |
1280 | memset(ioctx, 0, sizeof(struct srpt_send_ioctx)); | 1280 | |
1281 | ioctx->ch = ch; | 1281 | if (!ioctx) |
1282 | return ioctx; | ||
1283 | |||
1284 | BUG_ON(ioctx->ch != ch); | ||
1282 | spin_lock_init(&ioctx->spinlock); | 1285 | spin_lock_init(&ioctx->spinlock); |
1283 | ioctx->state = SRPT_STATE_NEW; | 1286 | ioctx->state = SRPT_STATE_NEW; |
1287 | ioctx->n_rbuf = 0; | ||
1288 | ioctx->rbufs = NULL; | ||
1289 | ioctx->n_rdma = 0; | ||
1290 | ioctx->n_rdma_wrs = 0; | ||
1291 | ioctx->rdma_wrs = NULL; | ||
1292 | ioctx->mapped_sg_count = 0; | ||
1284 | init_completion(&ioctx->tx_done); | 1293 | init_completion(&ioctx->tx_done); |
1285 | 1294 | ioctx->queue_status_only = false; | |
1286 | ioctx->cmd.map_tag = tag; | 1295 | /* |
1296 | * transport_init_se_cmd() does not initialize all fields, so do it | ||
1297 | * here. | ||
1298 | */ | ||
1299 | memset(&ioctx->cmd, 0, sizeof(ioctx->cmd)); | ||
1300 | memset(&ioctx->sense_data, 0, sizeof(ioctx->sense_data)); | ||
1287 | 1301 | ||
1288 | return ioctx; | 1302 | return ioctx; |
1289 | } | 1303 | } |
@@ -2021,7 +2035,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id, | |||
2021 | struct ib_cm_rep_param *rep_param; | 2035 | struct ib_cm_rep_param *rep_param; |
2022 | struct srpt_rdma_ch *ch, *tmp_ch; | 2036 | struct srpt_rdma_ch *ch, *tmp_ch; |
2023 | u32 it_iu_len; | 2037 | u32 it_iu_len; |
2024 | int ret = 0; | 2038 | int i, ret = 0; |
2025 | unsigned char *p; | 2039 | unsigned char *p; |
2026 | 2040 | ||
2027 | WARN_ON_ONCE(irqs_disabled()); | 2041 | WARN_ON_ONCE(irqs_disabled()); |
@@ -2143,6 +2157,12 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id, | |||
2143 | if (!ch->ioctx_ring) | 2157 | if (!ch->ioctx_ring) |
2144 | goto free_ch; | 2158 | goto free_ch; |
2145 | 2159 | ||
2160 | INIT_LIST_HEAD(&ch->free_list); | ||
2161 | for (i = 0; i < ch->rq_size; i++) { | ||
2162 | ch->ioctx_ring[i]->ch = ch; | ||
2163 | list_add_tail(&ch->ioctx_ring[i]->free_list, &ch->free_list); | ||
2164 | } | ||
2165 | |||
2146 | ret = srpt_create_ch_ib(ch); | 2166 | ret = srpt_create_ch_ib(ch); |
2147 | if (ret) { | 2167 | if (ret) { |
2148 | rej->reason = cpu_to_be32( | 2168 | rej->reason = cpu_to_be32( |
@@ -2173,8 +2193,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id, | |||
2173 | p = &ch->sess_name[0]; | 2193 | p = &ch->sess_name[0]; |
2174 | 2194 | ||
2175 | try_again: | 2195 | try_again: |
2176 | ch->sess = target_alloc_session(&sport->port_tpg_1, ch->rq_size, | 2196 | ch->sess = target_alloc_session(&sport->port_tpg_1, 0, 0, |
2177 | sizeof(struct srpt_send_ioctx), | ||
2178 | TARGET_PROT_NORMAL, p, ch, NULL); | 2197 | TARGET_PROT_NORMAL, p, ch, NULL); |
2179 | if (IS_ERR(ch->sess)) { | 2198 | if (IS_ERR(ch->sess)) { |
2180 | pr_info("Rejected login because no ACL has been" | 2199 | pr_info("Rejected login because no ACL has been" |
@@ -2881,7 +2900,7 @@ static void srpt_release_cmd(struct se_cmd *se_cmd) | |||
2881 | struct srpt_send_ioctx *ioctx = container_of(se_cmd, | 2900 | struct srpt_send_ioctx *ioctx = container_of(se_cmd, |
2882 | struct srpt_send_ioctx, cmd); | 2901 | struct srpt_send_ioctx, cmd); |
2883 | struct srpt_rdma_ch *ch = ioctx->ch; | 2902 | struct srpt_rdma_ch *ch = ioctx->ch; |
2884 | struct se_session *se_sess = ch->sess; | 2903 | unsigned long flags; |
2885 | 2904 | ||
2886 | WARN_ON(ioctx->state != SRPT_STATE_DONE); | 2905 | WARN_ON(ioctx->state != SRPT_STATE_DONE); |
2887 | WARN_ON(ioctx->mapped_sg_count != 0); | 2906 | WARN_ON(ioctx->mapped_sg_count != 0); |
@@ -2892,7 +2911,9 @@ static void srpt_release_cmd(struct se_cmd *se_cmd) | |||
2892 | ioctx->n_rbuf = 0; | 2911 | ioctx->n_rbuf = 0; |
2893 | } | 2912 | } |
2894 | 2913 | ||
2895 | percpu_ida_free(&se_sess->sess_tag_pool, se_cmd->map_tag); | 2914 | spin_lock_irqsave(&ch->spinlock, flags); |
2915 | list_add(&ioctx->free_list, &ch->free_list); | ||
2916 | spin_unlock_irqrestore(&ch->spinlock, flags); | ||
2896 | } | 2917 | } |
2897 | 2918 | ||
2898 | /** | 2919 | /** |
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h index ca288f019315..af9b8b527340 100644 --- a/drivers/infiniband/ulp/srpt/ib_srpt.h +++ b/drivers/infiniband/ulp/srpt/ib_srpt.h | |||
@@ -179,6 +179,7 @@ struct srpt_recv_ioctx { | |||
179 | * struct srpt_send_ioctx - SRPT send I/O context. | 179 | * struct srpt_send_ioctx - SRPT send I/O context. |
180 | * @ioctx: See above. | 180 | * @ioctx: See above. |
181 | * @ch: Channel pointer. | 181 | * @ch: Channel pointer. |
182 | * @free_list: Node in srpt_rdma_ch.free_list. | ||
182 | * @n_rbuf: Number of data buffers in the received SRP command. | 183 | * @n_rbuf: Number of data buffers in the received SRP command. |
183 | * @rbufs: Pointer to SRP data buffer array. | 184 | * @rbufs: Pointer to SRP data buffer array. |
184 | * @single_rbuf: SRP data buffer if the command has only a single buffer. | 185 | * @single_rbuf: SRP data buffer if the command has only a single buffer. |
@@ -201,6 +202,7 @@ struct srpt_send_ioctx { | |||
201 | struct srp_direct_buf *rbufs; | 202 | struct srp_direct_buf *rbufs; |
202 | struct srp_direct_buf single_rbuf; | 203 | struct srp_direct_buf single_rbuf; |
203 | struct scatterlist *sg; | 204 | struct scatterlist *sg; |
205 | struct list_head free_list; | ||
204 | spinlock_t spinlock; | 206 | spinlock_t spinlock; |
205 | enum srpt_command_state state; | 207 | enum srpt_command_state state; |
206 | struct se_cmd cmd; | 208 | struct se_cmd cmd; |