Skip to content

Commit 3c96888

Browse files
Bart Van Asschetorvalds
authored andcommitted
Revert "ib_srpt: Convert to percpu_ida tag allocation"
This reverts commit 0fd1072. 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>
1 parent 93061f3 commit 3c96888

File tree

2 files changed

+40
-17
lines changed

2 files changed

+40
-17
lines changed

drivers/infiniband/ulp/srpt/ib_srpt.c

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,26 +1264,40 @@ static int srpt_map_sg_to_ib_sge(struct srpt_rdma_ch *ch,
12641264
*/
12651265
static struct srpt_send_ioctx *srpt_get_send_ioctx(struct srpt_rdma_ch *ch)
12661266
{
1267-
struct se_session *se_sess;
12681267
struct srpt_send_ioctx *ioctx;
1269-
int tag;
1268+
unsigned long flags;
12701269

12711270
BUG_ON(!ch);
1272-
se_sess = ch->sess;
12731271

1274-
tag = percpu_ida_alloc(&se_sess->sess_tag_pool, TASK_RUNNING);
1275-
if (tag < 0) {
1276-
pr_err("Unable to obtain tag for srpt_send_ioctx\n");
1277-
return NULL;
1272+
ioctx = NULL;
1273+
spin_lock_irqsave(&ch->spinlock, flags);
1274+
if (!list_empty(&ch->free_list)) {
1275+
ioctx = list_first_entry(&ch->free_list,
1276+
struct srpt_send_ioctx, free_list);
1277+
list_del(&ioctx->free_list);
12781278
}
1279-
ioctx = &((struct srpt_send_ioctx *)se_sess->sess_cmd_map)[tag];
1280-
memset(ioctx, 0, sizeof(struct srpt_send_ioctx));
1281-
ioctx->ch = ch;
1279+
spin_unlock_irqrestore(&ch->spinlock, flags);
1280+
1281+
if (!ioctx)
1282+
return ioctx;
1283+
1284+
BUG_ON(ioctx->ch != ch);
12821285
spin_lock_init(&ioctx->spinlock);
12831286
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;
12841293
init_completion(&ioctx->tx_done);
1285-
1286-
ioctx->cmd.map_tag = tag;
1294+
ioctx->queue_status_only = false;
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));
12871301

12881302
return ioctx;
12891303
}
@@ -2021,7 +2035,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
20212035
struct ib_cm_rep_param *rep_param;
20222036
struct srpt_rdma_ch *ch, *tmp_ch;
20232037
u32 it_iu_len;
2024-
int ret = 0;
2038+
int i, ret = 0;
20252039
unsigned char *p;
20262040

20272041
WARN_ON_ONCE(irqs_disabled());
@@ -2143,6 +2157,12 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
21432157
if (!ch->ioctx_ring)
21442158
goto free_ch;
21452159

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+
21462166
ret = srpt_create_ch_ib(ch);
21472167
if (ret) {
21482168
rej->reason = cpu_to_be32(
@@ -2173,8 +2193,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
21732193
p = &ch->sess_name[0];
21742194

21752195
try_again:
2176-
ch->sess = target_alloc_session(&sport->port_tpg_1, ch->rq_size,
2177-
sizeof(struct srpt_send_ioctx),
2196+
ch->sess = target_alloc_session(&sport->port_tpg_1, 0, 0,
21782197
TARGET_PROT_NORMAL, p, ch, NULL);
21792198
if (IS_ERR(ch->sess)) {
21802199
pr_info("Rejected login because no ACL has been"
@@ -2881,7 +2900,7 @@ static void srpt_release_cmd(struct se_cmd *se_cmd)
28812900
struct srpt_send_ioctx *ioctx = container_of(se_cmd,
28822901
struct srpt_send_ioctx, cmd);
28832902
struct srpt_rdma_ch *ch = ioctx->ch;
2884-
struct se_session *se_sess = ch->sess;
2903+
unsigned long flags;
28852904

28862905
WARN_ON(ioctx->state != SRPT_STATE_DONE);
28872906
WARN_ON(ioctx->mapped_sg_count != 0);
@@ -2892,7 +2911,9 @@ static void srpt_release_cmd(struct se_cmd *se_cmd)
28922911
ioctx->n_rbuf = 0;
28932912
}
28942913

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);
28962917
}
28972918

28982919
/**

drivers/infiniband/ulp/srpt/ib_srpt.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ struct srpt_recv_ioctx {
179179
* struct srpt_send_ioctx - SRPT send I/O context.
180180
* @ioctx: See above.
181181
* @ch: Channel pointer.
182+
* @free_list: Node in srpt_rdma_ch.free_list.
182183
* @n_rbuf: Number of data buffers in the received SRP command.
183184
* @rbufs: Pointer to SRP data buffer array.
184185
* @single_rbuf: SRP data buffer if the command has only a single buffer.
@@ -201,6 +202,7 @@ struct srpt_send_ioctx {
201202
struct srp_direct_buf *rbufs;
202203
struct srp_direct_buf single_rbuf;
203204
struct scatterlist *sg;
205+
struct list_head free_list;
204206
spinlock_t spinlock;
205207
enum srpt_command_state state;
206208
struct se_cmd cmd;

0 commit comments

Comments
 (0)