Commit a94a2572 authored by Xiubo Li's avatar Xiubo Li Committed by Martin K. Petersen

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: default avatarXiubo Li <xiubli@redhat.com>
Acked-by: default avatarMike Christie <mchristi@redhat.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent cc29a1b0
...@@ -148,7 +148,7 @@ struct tcmu_dev { ...@@ -148,7 +148,7 @@ struct tcmu_dev {
size_t ring_size; size_t ring_size;
struct mutex cmdr_lock; struct mutex cmdr_lock;
struct list_head cmdr_queue; struct list_head qfull_queue;
uint32_t dbi_max; uint32_t dbi_max;
uint32_t dbi_thresh; uint32_t dbi_thresh;
...@@ -159,6 +159,7 @@ struct tcmu_dev { ...@@ -159,6 +159,7 @@ struct tcmu_dev {
struct timer_list cmd_timer; struct timer_list cmd_timer;
unsigned int cmd_time_out; unsigned int cmd_time_out;
struct list_head inflight_queue;
struct timer_list qfull_timer; struct timer_list qfull_timer;
int qfull_time_out; int qfull_time_out;
...@@ -179,7 +180,7 @@ struct tcmu_dev { ...@@ -179,7 +180,7 @@ struct tcmu_dev {
struct tcmu_cmd { struct tcmu_cmd {
struct se_cmd *se_cmd; struct se_cmd *se_cmd;
struct tcmu_dev *tcmu_dev; struct tcmu_dev *tcmu_dev;
struct list_head cmdr_queue_entry; struct list_head queue_entry;
uint16_t cmd_id; uint16_t cmd_id;
...@@ -192,6 +193,7 @@ struct tcmu_cmd { ...@@ -192,6 +193,7 @@ struct tcmu_cmd {
unsigned long deadline; unsigned long deadline;
#define TCMU_CMD_BIT_EXPIRED 0 #define TCMU_CMD_BIT_EXPIRED 0
#define TCMU_CMD_BIT_INFLIGHT 1
unsigned long flags; unsigned long flags;
}; };
/* /*
...@@ -586,7 +588,7 @@ static struct tcmu_cmd *tcmu_alloc_cmd(struct se_cmd *se_cmd) ...@@ -586,7 +588,7 @@ static struct tcmu_cmd *tcmu_alloc_cmd(struct se_cmd *se_cmd)
if (!tcmu_cmd) if (!tcmu_cmd)
return NULL; return NULL;
INIT_LIST_HEAD(&tcmu_cmd->cmdr_queue_entry); INIT_LIST_HEAD(&tcmu_cmd->queue_entry);
tcmu_cmd->se_cmd = se_cmd; tcmu_cmd->se_cmd = se_cmd;
tcmu_cmd->tcmu_dev = udev; tcmu_cmd->tcmu_dev = udev;
...@@ -915,11 +917,13 @@ static int tcmu_setup_cmd_timer(struct tcmu_cmd *tcmu_cmd, unsigned int tmo, ...@@ -915,11 +917,13 @@ static int tcmu_setup_cmd_timer(struct tcmu_cmd *tcmu_cmd, unsigned int tmo,
return 0; return 0;
tcmu_cmd->deadline = round_jiffies_up(jiffies + msecs_to_jiffies(tmo)); tcmu_cmd->deadline = round_jiffies_up(jiffies + msecs_to_jiffies(tmo));
if (!timer_pending(timer))
mod_timer(timer, tcmu_cmd->deadline); mod_timer(timer, tcmu_cmd->deadline);
return 0; return 0;
} }
static int add_to_cmdr_queue(struct tcmu_cmd *tcmu_cmd) static int add_to_qfull_queue(struct tcmu_cmd *tcmu_cmd)
{ {
struct tcmu_dev *udev = tcmu_cmd->tcmu_dev; struct tcmu_dev *udev = tcmu_cmd->tcmu_dev;
unsigned int tmo; unsigned int tmo;
...@@ -942,7 +946,7 @@ static int add_to_cmdr_queue(struct tcmu_cmd *tcmu_cmd) ...@@ -942,7 +946,7 @@ static int add_to_cmdr_queue(struct tcmu_cmd *tcmu_cmd)
if (ret) if (ret)
return ret; return ret;
list_add_tail(&tcmu_cmd->cmdr_queue_entry, &udev->cmdr_queue); list_add_tail(&tcmu_cmd->queue_entry, &udev->qfull_queue);
pr_debug("adding cmd %u on dev %s to ring space wait queue\n", pr_debug("adding cmd %u on dev %s to ring space wait queue\n",
tcmu_cmd->cmd_id, udev->name); tcmu_cmd->cmd_id, udev->name);
return 0; return 0;
...@@ -999,7 +1003,7 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err) ...@@ -999,7 +1003,7 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err)
base_command_size = tcmu_cmd_get_base_cmd_size(tcmu_cmd->dbi_cnt); base_command_size = tcmu_cmd_get_base_cmd_size(tcmu_cmd->dbi_cnt);
command_size = tcmu_cmd_get_cmd_size(tcmu_cmd, base_command_size); command_size = tcmu_cmd_get_cmd_size(tcmu_cmd, base_command_size);
if (!list_empty(&udev->cmdr_queue)) if (!list_empty(&udev->qfull_queue))
goto queue; goto queue;
mb = udev->mb_addr; mb = udev->mb_addr;
...@@ -1096,13 +1100,16 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err) ...@@ -1096,13 +1100,16 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err)
UPDATE_HEAD(mb->cmd_head, command_size, udev->cmdr_size); UPDATE_HEAD(mb->cmd_head, command_size, udev->cmdr_size);
tcmu_flush_dcache_range(mb, sizeof(*mb)); tcmu_flush_dcache_range(mb, sizeof(*mb));
list_add_tail(&tcmu_cmd->queue_entry, &udev->inflight_queue);
set_bit(TCMU_CMD_BIT_INFLIGHT, &tcmu_cmd->flags);
/* TODO: only if FLUSH and FUA? */ /* TODO: only if FLUSH and FUA? */
uio_event_notify(&udev->uio_info); uio_event_notify(&udev->uio_info);
return 0; return 0;
queue: queue:
if (add_to_cmdr_queue(tcmu_cmd)) { if (add_to_qfull_queue(tcmu_cmd)) {
*scsi_err = TCM_OUT_OF_RESOURCES; *scsi_err = TCM_OUT_OF_RESOURCES;
return -1; return -1;
} }
...@@ -1145,6 +1152,8 @@ static void tcmu_handle_completion(struct tcmu_cmd *cmd, struct tcmu_cmd_entry * ...@@ -1145,6 +1152,8 @@ static void tcmu_handle_completion(struct tcmu_cmd *cmd, struct tcmu_cmd_entry *
if (test_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags)) if (test_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags))
goto out; goto out;
list_del_init(&cmd->queue_entry);
tcmu_cmd_reset_dbi_cur(cmd); tcmu_cmd_reset_dbi_cur(cmd);
if (entry->hdr.uflags & TCMU_UFLAG_UNKNOWN_OP) { 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 * ...@@ -1194,9 +1203,29 @@ static void tcmu_handle_completion(struct tcmu_cmd *cmd, struct tcmu_cmd_entry *
tcmu_free_cmd(cmd); tcmu_free_cmd(cmd);
} }
static void tcmu_set_next_deadline(struct list_head *queue,
struct timer_list *timer)
{
struct tcmu_cmd *tcmu_cmd, *tmp_cmd;
unsigned long deadline = 0;
list_for_each_entry_safe(tcmu_cmd, tmp_cmd, queue, queue_entry) {
if (!time_after(jiffies, tcmu_cmd->deadline)) {
deadline = tcmu_cmd->deadline;
break;
}
}
if (deadline)
mod_timer(timer, deadline);
else
del_timer(timer);
}
static unsigned int tcmu_handle_completions(struct tcmu_dev *udev) static unsigned int tcmu_handle_completions(struct tcmu_dev *udev)
{ {
struct tcmu_mailbox *mb; struct tcmu_mailbox *mb;
struct tcmu_cmd *cmd;
int handled = 0; int handled = 0;
if (test_bit(TCMU_DEV_BIT_BROKEN, &udev->flags)) { if (test_bit(TCMU_DEV_BIT_BROKEN, &udev->flags)) {
...@@ -1210,7 +1239,6 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev) ...@@ -1210,7 +1239,6 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev)
while (udev->cmdr_last_cleaned != READ_ONCE(mb->cmd_tail)) { while (udev->cmdr_last_cleaned != READ_ONCE(mb->cmd_tail)) {
struct tcmu_cmd_entry *entry = (void *) mb + CMDR_OFF + udev->cmdr_last_cleaned; struct tcmu_cmd_entry *entry = (void *) mb + CMDR_OFF + udev->cmdr_last_cleaned;
struct tcmu_cmd *cmd;
tcmu_flush_dcache_range(entry, sizeof(*entry)); tcmu_flush_dcache_range(entry, sizeof(*entry));
...@@ -1243,7 +1271,7 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev) ...@@ -1243,7 +1271,7 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev)
/* no more pending commands */ /* no more pending commands */
del_timer(&udev->cmd_timer); del_timer(&udev->cmd_timer);
if (list_empty(&udev->cmdr_queue)) { if (list_empty(&udev->qfull_queue)) {
/* /*
* no more pending or waiting commands so try to * no more pending or waiting commands so try to
* reclaim blocks if needed. * reclaim blocks if needed.
...@@ -1252,6 +1280,8 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev) ...@@ -1252,6 +1280,8 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev)
tcmu_global_max_blocks) tcmu_global_max_blocks)
schedule_delayed_work(&tcmu_unmap_work, 0); schedule_delayed_work(&tcmu_unmap_work, 0);
} }
} else if (udev->cmd_time_out) {
tcmu_set_next_deadline(&udev->inflight_queue, &udev->cmd_timer);
} }
return handled; return handled;
...@@ -1271,7 +1301,7 @@ static int tcmu_check_expired_cmd(int id, void *p, void *data) ...@@ -1271,7 +1301,7 @@ static int tcmu_check_expired_cmd(int id, void *p, void *data)
if (!time_after(jiffies, cmd->deadline)) if (!time_after(jiffies, cmd->deadline))
return 0; return 0;
is_running = list_empty(&cmd->cmdr_queue_entry); is_running = test_bit(TCMU_CMD_BIT_INFLIGHT, &cmd->flags);
se_cmd = cmd->se_cmd; se_cmd = cmd->se_cmd;
if (is_running) { if (is_running) {
...@@ -1288,12 +1318,11 @@ static int tcmu_check_expired_cmd(int id, void *p, void *data) ...@@ -1288,12 +1318,11 @@ static int tcmu_check_expired_cmd(int id, void *p, void *data)
*/ */
scsi_status = SAM_STAT_CHECK_CONDITION; scsi_status = SAM_STAT_CHECK_CONDITION;
} else { } else {
list_del_init(&cmd->cmdr_queue_entry);
idr_remove(&udev->commands, id); idr_remove(&udev->commands, id);
tcmu_free_cmd(cmd); tcmu_free_cmd(cmd);
scsi_status = SAM_STAT_TASK_SET_FULL; scsi_status = SAM_STAT_TASK_SET_FULL;
} }
list_del_init(&cmd->queue_entry);
pr_debug("Timing out cmd %u on dev %s that is %s.\n", pr_debug("Timing out cmd %u on dev %s that is %s.\n",
id, udev->name, is_running ? "inflight" : "queued"); 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) ...@@ -1372,7 +1401,8 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name)
INIT_LIST_HEAD(&udev->node); INIT_LIST_HEAD(&udev->node);
INIT_LIST_HEAD(&udev->timedout_entry); INIT_LIST_HEAD(&udev->timedout_entry);
INIT_LIST_HEAD(&udev->cmdr_queue); INIT_LIST_HEAD(&udev->qfull_queue);
INIT_LIST_HEAD(&udev->inflight_queue);
idr_init(&udev->commands); idr_init(&udev->commands);
timer_setup(&udev->qfull_timer, tcmu_qfull_timedout, 0); timer_setup(&udev->qfull_timer, tcmu_qfull_timedout, 0);
...@@ -1383,7 +1413,7 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name) ...@@ -1383,7 +1413,7 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name)
return &udev->se_dev; return &udev->se_dev;
} }
static bool run_cmdr_queue(struct tcmu_dev *udev, bool fail) static bool run_qfull_queue(struct tcmu_dev *udev, bool fail)
{ {
struct tcmu_cmd *tcmu_cmd, *tmp_cmd; struct tcmu_cmd *tcmu_cmd, *tmp_cmd;
LIST_HEAD(cmds); LIST_HEAD(cmds);
...@@ -1391,15 +1421,15 @@ static bool run_cmdr_queue(struct tcmu_dev *udev, bool fail) ...@@ -1391,15 +1421,15 @@ static bool run_cmdr_queue(struct tcmu_dev *udev, bool fail)
sense_reason_t scsi_ret; sense_reason_t scsi_ret;
int ret; int ret;
if (list_empty(&udev->cmdr_queue)) if (list_empty(&udev->qfull_queue))
return true; return true;
pr_debug("running %s's cmdr queue forcefail %d\n", udev->name, fail); pr_debug("running %s's cmdr queue forcefail %d\n", udev->name, fail);
list_splice_init(&udev->cmdr_queue, &cmds); list_splice_init(&udev->qfull_queue, &cmds);
list_for_each_entry_safe(tcmu_cmd, tmp_cmd, &cmds, cmdr_queue_entry) { list_for_each_entry_safe(tcmu_cmd, tmp_cmd, &cmds, queue_entry) {
list_del_init(&tcmu_cmd->cmdr_queue_entry); list_del_init(&tcmu_cmd->queue_entry);
pr_debug("removing cmd %u on dev %s from queue\n", pr_debug("removing cmd %u on dev %s from queue\n",
tcmu_cmd->cmd_id, udev->name); tcmu_cmd->cmd_id, udev->name);
...@@ -1437,14 +1467,13 @@ static bool run_cmdr_queue(struct tcmu_dev *udev, bool fail) ...@@ -1437,14 +1467,13 @@ static bool run_cmdr_queue(struct tcmu_dev *udev, bool fail)
* cmd was requeued, so just put all cmds back in * cmd was requeued, so just put all cmds back in
* the queue * the queue
*/ */
list_splice_tail(&cmds, &udev->cmdr_queue); list_splice_tail(&cmds, &udev->qfull_queue);
drained = false; drained = false;
goto done; break;
} }
} }
if (list_empty(&udev->cmdr_queue))
del_timer(&udev->qfull_timer); tcmu_set_next_deadline(&udev->qfull_queue, &udev->qfull_timer);
done:
return drained; return drained;
} }
...@@ -1454,7 +1483,7 @@ static int tcmu_irqcontrol(struct uio_info *info, s32 irq_on) ...@@ -1454,7 +1483,7 @@ static int tcmu_irqcontrol(struct uio_info *info, s32 irq_on)
mutex_lock(&udev->cmdr_lock); mutex_lock(&udev->cmdr_lock);
tcmu_handle_completions(udev); tcmu_handle_completions(udev);
run_cmdr_queue(udev, false); run_qfull_queue(udev, false);
mutex_unlock(&udev->cmdr_lock); mutex_unlock(&udev->cmdr_lock);
return 0; return 0;
...@@ -1982,7 +2011,7 @@ static void tcmu_block_dev(struct tcmu_dev *udev) ...@@ -1982,7 +2011,7 @@ static void tcmu_block_dev(struct tcmu_dev *udev)
/* complete IO that has executed successfully */ /* complete IO that has executed successfully */
tcmu_handle_completions(udev); tcmu_handle_completions(udev);
/* fail IO waiting to be queued */ /* fail IO waiting to be queued */
run_cmdr_queue(udev, true); run_qfull_queue(udev, true);
unlock: unlock:
mutex_unlock(&udev->cmdr_lock); mutex_unlock(&udev->cmdr_lock);
...@@ -1997,7 +2026,7 @@ static void tcmu_reset_ring(struct tcmu_dev *udev, u8 err_level) ...@@ -1997,7 +2026,7 @@ static void tcmu_reset_ring(struct tcmu_dev *udev, u8 err_level)
mutex_lock(&udev->cmdr_lock); mutex_lock(&udev->cmdr_lock);
idr_for_each_entry(&udev->commands, cmd, i) { idr_for_each_entry(&udev->commands, cmd, i) {
if (!list_empty(&cmd->cmdr_queue_entry)) if (!test_bit(TCMU_CMD_BIT_INFLIGHT, &cmd->flags))
continue; continue;
pr_debug("removing cmd %u on dev %s from ring (is expired %d)\n", 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) ...@@ -2006,6 +2035,7 @@ static void tcmu_reset_ring(struct tcmu_dev *udev, u8 err_level)
idr_remove(&udev->commands, i); idr_remove(&udev->commands, i);
if (!test_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags)) { if (!test_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags)) {
list_del_init(&cmd->queue_entry);
if (err_level == 1) { if (err_level == 1) {
/* /*
* Userspace was not able to start the * Userspace was not able to start the
...@@ -2666,6 +2696,10 @@ static void check_timedout_devices(void) ...@@ -2666,6 +2696,10 @@ static void check_timedout_devices(void)
mutex_lock(&udev->cmdr_lock); mutex_lock(&udev->cmdr_lock);
idr_for_each(&udev->commands, tcmu_check_expired_cmd, NULL); idr_for_each(&udev->commands, tcmu_check_expired_cmd, NULL);
tcmu_set_next_deadline(&udev->inflight_queue, &udev->cmd_timer);
tcmu_set_next_deadline(&udev->qfull_queue, &udev->qfull_timer);
mutex_unlock(&udev->cmdr_lock); mutex_unlock(&udev->cmdr_lock);
spin_lock_bh(&timed_out_udevs_lock); spin_lock_bh(&timed_out_udevs_lock);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment