Skip to content

Commit a94a257

Browse files
lxbszmartinkpetersen
lxbsz
authored and
martinkpetersen
committed
scsi: tcmu: avoid cmd/qfull timers updated whenever a new cmd comes
Currently there is one cmd timeout timer and one qfull timer for each udev, and whenever any new command is coming in we will update the cmd timer or qfull timer. For some corner cases the timers are always working only for the ringbuffer's and full queue's newest cmd. That's to say the timer won't be fired even if one cmd has been stuck for a very long time and the deadline is reached. This fix will keep the cmd/qfull timers to be pended for the oldest cmd in ringbuffer and full queue, and will update them with the next cmd's deadline only when the old cmd's deadline is reached or removed from the ringbuffer and full queue. Signed-off-by: Xiubo Li <xiubli@redhat.com> Acked-by: Mike Christie <mchristi@redhat.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent cc29a1b commit a94a257

File tree

1 file changed

+61
-27
lines changed

1 file changed

+61
-27
lines changed

‎drivers/target/target_core_user.c

Lines changed: 61 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ struct tcmu_dev {
148148
size_t ring_size;
149149

150150
struct mutex cmdr_lock;
151-
struct list_head cmdr_queue;
151+
struct list_head qfull_queue;
152152

153153
uint32_t dbi_max;
154154
uint32_t dbi_thresh;
@@ -159,6 +159,7 @@ struct tcmu_dev {
159159

160160
struct timer_list cmd_timer;
161161
unsigned int cmd_time_out;
162+
struct list_head inflight_queue;
162163

163164
struct timer_list qfull_timer;
164165
int qfull_time_out;
@@ -179,7 +180,7 @@ struct tcmu_dev {
179180
struct tcmu_cmd {
180181
struct se_cmd *se_cmd;
181182
struct tcmu_dev *tcmu_dev;
182-
struct list_head cmdr_queue_entry;
183+
struct list_head queue_entry;
183184

184185
uint16_t cmd_id;
185186

@@ -192,6 +193,7 @@ struct tcmu_cmd {
192193
unsigned long deadline;
193194

194195
#define TCMU_CMD_BIT_EXPIRED 0
196+
#define TCMU_CMD_BIT_INFLIGHT 1
195197
unsigned long flags;
196198
};
197199
/*
@@ -586,7 +588,7 @@ static struct tcmu_cmd *tcmu_alloc_cmd(struct se_cmd *se_cmd)
586588
if (!tcmu_cmd)
587589
return NULL;
588590

589-
INIT_LIST_HEAD(&tcmu_cmd->cmdr_queue_entry);
591+
INIT_LIST_HEAD(&tcmu_cmd->queue_entry);
590592
tcmu_cmd->se_cmd = se_cmd;
591593
tcmu_cmd->tcmu_dev = udev;
592594

@@ -915,11 +917,13 @@ static int tcmu_setup_cmd_timer(struct tcmu_cmd *tcmu_cmd, unsigned int tmo,
915917
return 0;
916918

917919
tcmu_cmd->deadline = round_jiffies_up(jiffies + msecs_to_jiffies(tmo));
918-
mod_timer(timer, tcmu_cmd->deadline);
920+
if (!timer_pending(timer))
921+
mod_timer(timer, tcmu_cmd->deadline);
922+
919923
return 0;
920924
}
921925

922-
static int add_to_cmdr_queue(struct tcmu_cmd *tcmu_cmd)
926+
static int add_to_qfull_queue(struct tcmu_cmd *tcmu_cmd)
923927
{
924928
struct tcmu_dev *udev = tcmu_cmd->tcmu_dev;
925929
unsigned int tmo;
@@ -942,7 +946,7 @@ static int add_to_cmdr_queue(struct tcmu_cmd *tcmu_cmd)
942946
if (ret)
943947
return ret;
944948

945-
list_add_tail(&tcmu_cmd->cmdr_queue_entry, &udev->cmdr_queue);
949+
list_add_tail(&tcmu_cmd->queue_entry, &udev->qfull_queue);
946950
pr_debug("adding cmd %u on dev %s to ring space wait queue\n",
947951
tcmu_cmd->cmd_id, udev->name);
948952
return 0;
@@ -999,7 +1003,7 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err)
9991003
base_command_size = tcmu_cmd_get_base_cmd_size(tcmu_cmd->dbi_cnt);
10001004
command_size = tcmu_cmd_get_cmd_size(tcmu_cmd, base_command_size);
10011005

1002-
if (!list_empty(&udev->cmdr_queue))
1006+
if (!list_empty(&udev->qfull_queue))
10031007
goto queue;
10041008

10051009
mb = udev->mb_addr;
@@ -1096,13 +1100,16 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err)
10961100
UPDATE_HEAD(mb->cmd_head, command_size, udev->cmdr_size);
10971101
tcmu_flush_dcache_range(mb, sizeof(*mb));
10981102

1103+
list_add_tail(&tcmu_cmd->queue_entry, &udev->inflight_queue);
1104+
set_bit(TCMU_CMD_BIT_INFLIGHT, &tcmu_cmd->flags);
1105+
10991106
/* TODO: only if FLUSH and FUA? */
11001107
uio_event_notify(&udev->uio_info);
11011108

11021109
return 0;
11031110

11041111
queue:
1105-
if (add_to_cmdr_queue(tcmu_cmd)) {
1112+
if (add_to_qfull_queue(tcmu_cmd)) {
11061113
*scsi_err = TCM_OUT_OF_RESOURCES;
11071114
return -1;
11081115
}
@@ -1145,6 +1152,8 @@ static void tcmu_handle_completion(struct tcmu_cmd *cmd, struct tcmu_cmd_entry *
11451152
if (test_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags))
11461153
goto out;
11471154

1155+
list_del_init(&cmd->queue_entry);
1156+
11481157
tcmu_cmd_reset_dbi_cur(cmd);
11491158

11501159
if (entry->hdr.uflags & TCMU_UFLAG_UNKNOWN_OP) {
@@ -1194,9 +1203,29 @@ static void tcmu_handle_completion(struct tcmu_cmd *cmd, struct tcmu_cmd_entry *
11941203
tcmu_free_cmd(cmd);
11951204
}
11961205

1206+
static void tcmu_set_next_deadline(struct list_head *queue,
1207+
struct timer_list *timer)
1208+
{
1209+
struct tcmu_cmd *tcmu_cmd, *tmp_cmd;
1210+
unsigned long deadline = 0;
1211+
1212+
list_for_each_entry_safe(tcmu_cmd, tmp_cmd, queue, queue_entry) {
1213+
if (!time_after(jiffies, tcmu_cmd->deadline)) {
1214+
deadline = tcmu_cmd->deadline;
1215+
break;
1216+
}
1217+
}
1218+
1219+
if (deadline)
1220+
mod_timer(timer, deadline);
1221+
else
1222+
del_timer(timer);
1223+
}
1224+
11971225
static unsigned int tcmu_handle_completions(struct tcmu_dev *udev)
11981226
{
11991227
struct tcmu_mailbox *mb;
1228+
struct tcmu_cmd *cmd;
12001229
int handled = 0;
12011230

12021231
if (test_bit(TCMU_DEV_BIT_BROKEN, &udev->flags)) {
@@ -1210,7 +1239,6 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev)
12101239
while (udev->cmdr_last_cleaned != READ_ONCE(mb->cmd_tail)) {
12111240

12121241
struct tcmu_cmd_entry *entry = (void *) mb + CMDR_OFF + udev->cmdr_last_cleaned;
1213-
struct tcmu_cmd *cmd;
12141242

12151243
tcmu_flush_dcache_range(entry, sizeof(*entry));
12161244

@@ -1243,7 +1271,7 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev)
12431271
/* no more pending commands */
12441272
del_timer(&udev->cmd_timer);
12451273

1246-
if (list_empty(&udev->cmdr_queue)) {
1274+
if (list_empty(&udev->qfull_queue)) {
12471275
/*
12481276
* no more pending or waiting commands so try to
12491277
* reclaim blocks if needed.
@@ -1252,6 +1280,8 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev)
12521280
tcmu_global_max_blocks)
12531281
schedule_delayed_work(&tcmu_unmap_work, 0);
12541282
}
1283+
} else if (udev->cmd_time_out) {
1284+
tcmu_set_next_deadline(&udev->inflight_queue, &udev->cmd_timer);
12551285
}
12561286

12571287
return handled;
@@ -1271,7 +1301,7 @@ static int tcmu_check_expired_cmd(int id, void *p, void *data)
12711301
if (!time_after(jiffies, cmd->deadline))
12721302
return 0;
12731303

1274-
is_running = list_empty(&cmd->cmdr_queue_entry);
1304+
is_running = test_bit(TCMU_CMD_BIT_INFLIGHT, &cmd->flags);
12751305
se_cmd = cmd->se_cmd;
12761306

12771307
if (is_running) {
@@ -1288,12 +1318,11 @@ static int tcmu_check_expired_cmd(int id, void *p, void *data)
12881318
*/
12891319
scsi_status = SAM_STAT_CHECK_CONDITION;
12901320
} else {
1291-
list_del_init(&cmd->cmdr_queue_entry);
1292-
12931321
idr_remove(&udev->commands, id);
12941322
tcmu_free_cmd(cmd);
12951323
scsi_status = SAM_STAT_TASK_SET_FULL;
12961324
}
1325+
list_del_init(&cmd->queue_entry);
12971326

12981327
pr_debug("Timing out cmd %u on dev %s that is %s.\n",
12991328
id, udev->name, is_running ? "inflight" : "queued");
@@ -1372,7 +1401,8 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name)
13721401

13731402
INIT_LIST_HEAD(&udev->node);
13741403
INIT_LIST_HEAD(&udev->timedout_entry);
1375-
INIT_LIST_HEAD(&udev->cmdr_queue);
1404+
INIT_LIST_HEAD(&udev->qfull_queue);
1405+
INIT_LIST_HEAD(&udev->inflight_queue);
13761406
idr_init(&udev->commands);
13771407

13781408
timer_setup(&udev->qfull_timer, tcmu_qfull_timedout, 0);
@@ -1383,23 +1413,23 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name)
13831413
return &udev->se_dev;
13841414
}
13851415

1386-
static bool run_cmdr_queue(struct tcmu_dev *udev, bool fail)
1416+
static bool run_qfull_queue(struct tcmu_dev *udev, bool fail)
13871417
{
13881418
struct tcmu_cmd *tcmu_cmd, *tmp_cmd;
13891419
LIST_HEAD(cmds);
13901420
bool drained = true;
13911421
sense_reason_t scsi_ret;
13921422
int ret;
13931423

1394-
if (list_empty(&udev->cmdr_queue))
1424+
if (list_empty(&udev->qfull_queue))
13951425
return true;
13961426

13971427
pr_debug("running %s's cmdr queue forcefail %d\n", udev->name, fail);
13981428

1399-
list_splice_init(&udev->cmdr_queue, &cmds);
1429+
list_splice_init(&udev->qfull_queue, &cmds);
14001430

1401-
list_for_each_entry_safe(tcmu_cmd, tmp_cmd, &cmds, cmdr_queue_entry) {
1402-
list_del_init(&tcmu_cmd->cmdr_queue_entry);
1431+
list_for_each_entry_safe(tcmu_cmd, tmp_cmd, &cmds, queue_entry) {
1432+
list_del_init(&tcmu_cmd->queue_entry);
14031433

14041434
pr_debug("removing cmd %u on dev %s from queue\n",
14051435
tcmu_cmd->cmd_id, udev->name);
@@ -1437,14 +1467,13 @@ static bool run_cmdr_queue(struct tcmu_dev *udev, bool fail)
14371467
* cmd was requeued, so just put all cmds back in
14381468
* the queue
14391469
*/
1440-
list_splice_tail(&cmds, &udev->cmdr_queue);
1470+
list_splice_tail(&cmds, &udev->qfull_queue);
14411471
drained = false;
1442-
goto done;
1472+
break;
14431473
}
14441474
}
1445-
if (list_empty(&udev->cmdr_queue))
1446-
del_timer(&udev->qfull_timer);
1447-
done:
1475+
1476+
tcmu_set_next_deadline(&udev->qfull_queue, &udev->qfull_timer);
14481477
return drained;
14491478
}
14501479

@@ -1454,7 +1483,7 @@ static int tcmu_irqcontrol(struct uio_info *info, s32 irq_on)
14541483

14551484
mutex_lock(&udev->cmdr_lock);
14561485
tcmu_handle_completions(udev);
1457-
run_cmdr_queue(udev, false);
1486+
run_qfull_queue(udev, false);
14581487
mutex_unlock(&udev->cmdr_lock);
14591488

14601489
return 0;
@@ -1982,7 +2011,7 @@ static void tcmu_block_dev(struct tcmu_dev *udev)
19822011
/* complete IO that has executed successfully */
19832012
tcmu_handle_completions(udev);
19842013
/* fail IO waiting to be queued */
1985-
run_cmdr_queue(udev, true);
2014+
run_qfull_queue(udev, true);
19862015

19872016
unlock:
19882017
mutex_unlock(&udev->cmdr_lock);
@@ -1997,7 +2026,7 @@ static void tcmu_reset_ring(struct tcmu_dev *udev, u8 err_level)
19972026
mutex_lock(&udev->cmdr_lock);
19982027

19992028
idr_for_each_entry(&udev->commands, cmd, i) {
2000-
if (!list_empty(&cmd->cmdr_queue_entry))
2029+
if (!test_bit(TCMU_CMD_BIT_INFLIGHT, &cmd->flags))
20012030
continue;
20022031

20032032
pr_debug("removing cmd %u on dev %s from ring (is expired %d)\n",
@@ -2006,6 +2035,7 @@ static void tcmu_reset_ring(struct tcmu_dev *udev, u8 err_level)
20062035

20072036
idr_remove(&udev->commands, i);
20082037
if (!test_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags)) {
2038+
list_del_init(&cmd->queue_entry);
20092039
if (err_level == 1) {
20102040
/*
20112041
* Userspace was not able to start the
@@ -2666,6 +2696,10 @@ static void check_timedout_devices(void)
26662696

26672697
mutex_lock(&udev->cmdr_lock);
26682698
idr_for_each(&udev->commands, tcmu_check_expired_cmd, NULL);
2699+
2700+
tcmu_set_next_deadline(&udev->inflight_queue, &udev->cmd_timer);
2701+
tcmu_set_next_deadline(&udev->qfull_queue, &udev->qfull_timer);
2702+
26692703
mutex_unlock(&udev->cmdr_lock);
26702704

26712705
spin_lock_bh(&timed_out_udevs_lock);

0 commit comments

Comments
 (0)