> +static void transport_add_tasks_from_cmd(struct se_cmd *cmd)
> +{
> + unsigned long flags;
> + struct se_device *dev = cmd->se_dev;
> +
> + spin_lock_irqsave(&dev->execute_task_lock, flags);
> + __transport_add_tasks_from_cmd(cmd);
> spin_unlock_irqrestore(&dev->execute_task_lock, flags);
> }
Given that there is only one caller of this one left I'd remove it an
opencode the locking there.
>
> @@ -2142,19 +2149,16 @@ static int transport_execute_tasks(struct se_cmd *cmd)
> if (!add_tasks)
> goto execute_tasks;
> /*
> + * __transport_execute_tasks() -> __transport_add_tasks_from_cmd()
> + * adds associated se_tasks while holding dev->execute_task_lock
> + * before I/O dispath to avoid a double spinlock access.
> */
I don't think this comment adds any value. If at all a
/* expects execute_task_lock to be held */
comment ontop of __transport_execute_tasks would be more useful.
> + __transport_execute_tasks(se_dev, cmd);
> + return 0;
> }
> +
> execute_tasks:
> + __transport_execute_tasks(se_dev, NULL);
> return 0;
> }
Does it still make any sense to run the queue if
transport_cmd_check_stop return true from the submission context?
> @@ -2164,7 +2168,7 @@ execute_tasks:
> *
> * Called from transport_processing_thread()
> */
That comment hasn't been true for a while, time to remove it.
> -static int __transport_execute_tasks(struct se_device *dev)
> +static int __transport_execute_tasks(struct se_device *dev, struct se_cmd *new_cmd)
over 8- charactres line.
> {
> int error;
> struct se_cmd *cmd = NULL;
> @@ -2176,8 +2180,10 @@ static int __transport_execute_tasks(struct se_device *dev)
> * set enforce_dev_tcq=0, and dispatch new tasks as soon as
> * possible.
> */
> -check_depth:
> spin_lock_irq(&dev->execute_task_lock);
> + if (new_cmd != NULL)
> + __transport_add_tasks_from_cmd(new_cmd);
> +
> if (dev->enforce_dev_tcq) {
> if (!dev->depth_left) {
> spin_unlock_irq(&dev->execute_task_lock);
> @@ -2225,8 +2231,6 @@ check_depth:
> transport_generic_request_failure(cmd);
> }
>
> - goto check_depth;
> -
What replacing the looping over multiple tasks? Both the initial task
submission and the target processing thread might have multiple tasks
to work on.
--
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
