On Wed, 2017-02-01 at 16:58 -0800, Bart Van Assche wrote:
> The function transport_cmd_check_stop() has two callers. These callers
> invoke this function as follows:
> * transport_cmd_check_stop(cmd, true, false)
> * transport_cmd_check_stop(cmd, false, true)
> Hence inline this function into its callers.
>
> This patch does not change any functionality but improves source
> code readability.
Nice simplification here. One comment below.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxxxxxx>
> Reviewed-by: Hannes Reinecke <hare@xxxxxxxx>
> Reviewed-by: Bryant G. Ly <bryantly@xxxxxxxxxxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Andy Grover <agrover@xxxxxxxxxx>
> Cc: David Disseldorp <ddiss@xxxxxxx>
> ---
> drivers/target/target_core_transport.c | 68 +++++++++++++++++-----------------
> include/target/target_core_fabric.h | 2 +-
> 2 files changed, 34 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index 25bc214a4eee..40dff4799984 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -602,24 +602,18 @@ static void target_remove_from_state_list(struct se_cmd *cmd)
> spin_unlock_irqrestore(&dev->execute_task_lock, flags);
> }
>
> -static int transport_cmd_check_stop(struct se_cmd *cmd, bool remove_from_lists,
> - bool write_pending)
> +static int transport_cmd_check_stop_to_fabric(struct se_cmd *cmd)
> {
> unsigned long flags;
>
> - if (remove_from_lists) {
> - target_remove_from_state_list(cmd);
> + target_remove_from_state_list(cmd);
>
> - /*
> - * Clear struct se_cmd->se_lun before the handoff to FE.
> - */
> - cmd->se_lun = NULL;
> - }
> + /*
> + * Clear struct se_cmd->se_lun before the handoff to FE.
> + */
> + cmd->se_lun = NULL;
>
> spin_lock_irqsave(&cmd->t_state_lock, flags);
> - if (write_pending)
> - cmd->t_state = TRANSPORT_WRITE_PENDING;
> -
> /*
> * Determine if frontend context caller is requesting the stopping of
> * this command for frontend exceptions.
> @@ -632,30 +626,17 @@ static int transport_cmd_check_stop(struct se_cmd *cmd, bool remove_from_lists,
> } else {
> cmd->transport_state &= ~CMD_T_ACTIVE;
> }
> -
> - if (remove_from_lists) {
> - /*
> - * Some fabric modules like tcm_loop can release
> - * their internally allocated I/O reference now and
> - * struct se_cmd now.
> - *
> - * Fabric modules are expected to return '1' here if the
> - * se_cmd being passed is released at this point,
> - * or zero if not being released.
> - */
> - if (cmd->se_tfo->check_stop_free != NULL) {
> - spin_unlock_irqrestore(&cmd->t_state_lock, flags);
> - return cmd->se_tfo->check_stop_free(cmd);
> - }
> - }
> -
> spin_unlock_irqrestore(&cmd->t_state_lock, flags);
> - return 0;
> -}
>
> -static int transport_cmd_check_stop_to_fabric(struct se_cmd *cmd)
> -{
> - return transport_cmd_check_stop(cmd, true, false);
> + /*
> + * Some fabric modules like tcm_loop can release their internally
> + * allocated I/O reference and struct se_cmd now.
> + *
> + * Fabric modules are expected to return '1' here if the se_cmd being
> + * passed is released at this point, or zero if not being released.
> + */
> + return cmd->se_tfo->check_stop_free ? cmd->se_tfo->check_stop_free(cmd)
> + : 0;
> }
I think all of the in-tree fabric drivers provide a ->check_stop_free()
callback.
It would make sense to go ahead and enforce it's existence tree-wide at
the target_fabric_tf_ops_check() level, and drop the extra conditional
check here.
--
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
