Currently we stop the timers for all tasks in a command fairly late during
I/O completion, which is fairly pointless and requires all kinds of safety
checks.
Instead delete pending timers early on in transport_complete_task, thus
ensuring no new timers firest after that. We take t_state_lock a bit later
in that function thus making sure currenly running timers are out of the
criticial section. To be completely sure the timer has finished we also
add another del_timer_sync call when freeing the task.
This also allows removing TF_TIMER_RUNNING as it would be equivalent
to TF_ACTIVE now.
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Index: lio-core/drivers/target/target_core_transport.c
===================================================================
--- lio-core.orig/drivers/target/target_core_transport.c 2011-10-17 15:26:19.555648770 +0200
+++ lio-core/drivers/target/target_core_transport.c 2011-10-17 15:34:11.383547597 +0200
@@ -85,7 +85,6 @@ static int transport_generic_get_mem(str
static void transport_put_cmd(struct se_cmd *cmd);
static void transport_remove_cmd_from_queue(struct se_cmd *cmd);
static int transport_set_sense_codes(struct se_cmd *cmd, u8 asc, u8 ascq);
-static void transport_stop_all_task_timers(struct se_cmd *cmd);
int init_se_kmem_caches(void)
{
@@ -715,6 +714,8 @@ void transport_complete_task(struct se_t
if (dev)
atomic_inc(&dev->depth_left);
+ del_timer(&task->task_timer);
+
spin_lock_irqsave(&cmd->t_state_lock, flags);
task->task_flags &= ~TF_ACTIVE;
@@ -1510,6 +1511,7 @@ transport_generic_get_task(struct se_cmd
INIT_LIST_HEAD(&task->t_list);
INIT_LIST_HEAD(&task->t_execute_list);
INIT_LIST_HEAD(&task->t_state_list);
+ init_timer(&task->task_timer);
init_completion(&task->task_stop_comp);
task->task_se_cmd = cmd;
task->task_data_direction = data_direction;
@@ -1778,6 +1780,7 @@ bool target_stop_task(struct se_task *ta
spin_unlock_irqrestore(&cmd->t_state_lock, *flags);
pr_debug("Task %p waiting to complete\n", task);
+ del_timer_sync(&task->task_timer);
wait_for_completion(&task->task_stop_comp);
pr_debug("Task %p stopped successfully\n", task);
@@ -1787,7 +1790,6 @@ bool target_stop_task(struct se_task *ta
was_active = true;
}
- __transport_stop_task_timer(task, flags);
return was_active;
}
@@ -1862,8 +1864,6 @@ static void transport_generic_request_fa
atomic_read(&cmd->t_transport_stop),
atomic_read(&cmd->t_transport_sent));
- transport_stop_all_task_timers(cmd);
-
if (dev)
atomic_inc(&dev->depth_left);
/*
@@ -2068,7 +2068,6 @@ static void transport_task_timeout_handl
pr_debug("transport task timeout fired! task: %p cmd: %p\n", task, cmd);
spin_lock_irqsave(&cmd->t_state_lock, flags);
- task->task_flags &= ~TF_TIMER_RUNNING;
/*
* Determine if transport_complete_task() has already been called.
@@ -2111,16 +2110,11 @@ static void transport_task_timeout_handl
transport_add_cmd_to_queue(cmd, TRANSPORT_COMPLETE_FAILURE, false);
}
-/*
- * Called with cmd->t_state_lock held.
- */
static void transport_start_task_timer(struct se_task *task)
{
struct se_device *dev = task->task_se_cmd->se_dev;
int timeout;
- if (task->task_flags & TF_TIMER_RUNNING)
- return;
/*
* If the task_timeout is disabled, exit now.
*/
@@ -2128,47 +2122,10 @@ static void transport_start_task_timer(s
if (!timeout)
return;
- init_timer(&task->task_timer);
task->task_timer.expires = (get_jiffies_64() + timeout * HZ);
task->task_timer.data = (unsigned long) task;
task->task_timer.function = transport_task_timeout_handler;
-
- task->task_flags |= TF_TIMER_RUNNING;
add_timer(&task->task_timer);
-#if 0
- pr_debug("Starting task timer for cmd: %p task: %p seconds:"
- " %d\n", task->task_se_cmd, task, timeout);
-#endif
-}
-
-/*
- * Called with spin_lock_irq(&cmd->t_state_lock) held.
- */
-void __transport_stop_task_timer(struct se_task *task, unsigned long *flags)
-{
- struct se_cmd *cmd = task->task_se_cmd;
-
- if (!(task->task_flags & TF_TIMER_RUNNING))
- return;
-
- spin_unlock_irqrestore(&cmd->t_state_lock, *flags);
-
- del_timer_sync(&task->task_timer);
-
- spin_lock_irqsave(&cmd->t_state_lock, *flags);
- task->task_flags &= ~TF_TIMER_RUNNING;
-}
-
-static void transport_stop_all_task_timers(struct se_cmd *cmd)
-{
- struct se_task *task = NULL, *task_tmp;
- unsigned long flags;
-
- spin_lock_irqsave(&cmd->t_state_lock, flags);
- list_for_each_entry_safe(task, task_tmp,
- &cmd->t_task_list, t_list)
- __transport_stop_task_timer(task, &flags);
- spin_unlock_irqrestore(&cmd->t_state_lock, flags);
}
static inline int transport_tcq_window_closed(struct se_device *dev)
@@ -2367,6 +2324,7 @@ check_depth:
spin_lock_irqsave(&cmd->t_state_lock, flags);
task->task_flags &= ~TF_ACTIVE;
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
+ del_timer_sync(&task->task_timer);
atomic_set(&cmd->transport_sent, 0);
transport_stop_tasks_for_cmd(cmd);
transport_generic_request_failure(cmd, dev, 0, 1);
@@ -2405,6 +2363,7 @@ check_depth:
spin_lock_irqsave(&cmd->t_state_lock, flags);
task->task_flags &= ~TF_ACTIVE;
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
+ del_timer_sync(&task->task_timer);
atomic_set(&cmd->transport_sent, 0);
transport_stop_tasks_for_cmd(cmd);
transport_generic_request_failure(cmd, dev, 0, 1);
@@ -3433,7 +3392,6 @@ static void transport_complete_qf(struct
{
int ret = 0;
- transport_stop_all_task_timers(cmd);
if (cmd->se_dev->dev_task_attr_type == SAM_TASK_ATTR_EMULATED)
transport_complete_task_attr(cmd);
@@ -3603,6 +3561,14 @@ static void transport_free_dev_tasks(str
while (!list_empty(&dispose_list)) {
task = list_first_entry(&dispose_list, struct se_task, t_list);
+ /*
+ * We already cancelled all pending timers in
+ * transport_complete_task, but that was just a pure del_timer,
+ * so do a full del_timer_sync here to make sure any handler
+ * that was running at that point has finished execution.
+ */
+ del_timer_sync(&task->task_timer);
+
kfree(task->task_sg_bidi);
kfree(task->task_sg);
@@ -4822,7 +4788,6 @@ get_cmd:
transport_generic_process_write(cmd);
break;
case TRANSPORT_COMPLETE_OK:
- transport_stop_all_task_timers(cmd);
transport_generic_complete_ok(cmd);
break;
case TRANSPORT_REMOVE:
@@ -4838,7 +4803,6 @@ get_cmd:
transport_generic_request_failure(cmd, NULL, 1, 1);
break;
case TRANSPORT_COMPLETE_TIMEOUT:
- transport_stop_all_task_timers(cmd);
transport_generic_request_timeout(cmd);
break;
case TRANSPORT_COMPLETE_QF_WP:
Index: lio-core/include/target/target_core_base.h
===================================================================
--- lio-core.orig/include/target/target_core_base.h 2011-10-17 15:22:08.812650883 +0200
+++ lio-core/include/target/target_core_base.h 2011-10-17 15:26:40.000150858 +0200
@@ -77,7 +77,6 @@ enum se_task_flags {
TF_SENT = (1 << 1),
TF_TIMEOUT = (1 << 2),
TF_REQUEST_STOP = (1 << 3),
- TF_TIMER_RUNNING = (1 << 4),
};
/* Special transport agnostic struct se_cmd->t_states */
Index: lio-core/include/target/target_core_transport.h
===================================================================
--- lio-core.orig/include/target/target_core_transport.h 2011-10-17 15:28:32.147650686 +0200
+++ lio-core/include/target/target_core_transport.h 2011-10-17 15:28:38.487648851 +0200
@@ -172,7 +172,6 @@ extern void transport_new_cmd_failure(st
extern int transport_generic_handle_tmr(struct se_cmd *);
extern void transport_generic_free_cmd_intr(struct se_cmd *);
extern bool target_stop_task(struct se_task *task, unsigned long *flags);
-extern void __transport_stop_task_timer(struct se_task *, unsigned long *);
extern int transport_generic_map_mem_to_cmd(struct se_cmd *cmd, struct scatterlist *, u32,
struct scatterlist *, u32);
extern int transport_clear_lun_from_sessions(struct se_lun *);
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
