On 05/05/2017 12:50 AM, Bart Van Assche wrote:
> Since .se_tfo is only set if a command has been submitted to
> the LIO core, check .se_tfo instead of .iscsi_opcode. Since
> __iscsit_free_cmd() only affects SCSI commands but not TMFs,
> calling that function for TMFs does not change behavior. This
> patch does not change the behavior of iscsit_free_cmd().
>
> Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxxxxxx>
> Cc: Hannes Reinecke <hare@xxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Andy Grover <agrover@xxxxxxxxxx>
> Cc: David Disseldorp <ddiss@xxxxxxx>
> ---
> drivers/target/iscsi/iscsi_target_util.c | 39 ++++----------------------------
> 1 file changed, 4 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
> index 41b9e7cc08b8..1e36f83b5961 100644
> --- a/drivers/target/iscsi/iscsi_target_util.c
> +++ b/drivers/target/iscsi/iscsi_target_util.c
> @@ -734,49 +734,18 @@ void __iscsit_free_cmd(struct iscsi_cmd *cmd, bool check_queues)
>
> void iscsit_free_cmd(struct iscsi_cmd *cmd, bool shutdown)
> {
> - struct se_cmd *se_cmd = NULL;
> + struct se_cmd *se_cmd = cmd->se_cmd.se_tfo ? &cmd->se_cmd : NULL;
> int rc;
>
> - /*
> - * Determine if a struct se_cmd is associated with
> - * this struct iscsi_cmd.
> - */
> - switch (cmd->iscsi_opcode) {
> - case ISCSI_OP_SCSI_CMD:
> - /*
> - * Fallthrough
> - */
> - case ISCSI_OP_SCSI_TMFUNC:
> - se_cmd = &cmd->se_cmd;
> - __iscsit_free_cmd(cmd, shutdown);
> + __iscsit_free_cmd(cmd, shutdown);
> + if (se_cmd) {
> rc = transport_generic_free_cmd(se_cmd, shutdown);
> if (!rc && shutdown && se_cmd->se_sess) {
> __iscsit_free_cmd(cmd, shutdown);
> target_put_sess_cmd(se_cmd);
> }
> - break;
> - case ISCSI_OP_REJECT:
> - /*
> - * Handle special case for REJECT when iscsi_add_reject*() has
> - * overwritten the original iscsi_opcode assignment, and the
> - * associated cmd->se_cmd needs to be released.
> - */
> - if (cmd->se_cmd.se_tfo != NULL) {
> - se_cmd = &cmd->se_cmd;
> - __iscsit_free_cmd(cmd, shutdown);
> -
> - rc = transport_generic_free_cmd(&cmd->se_cmd, shutdown);
> - if (!rc && shutdown && se_cmd->se_sess) {
> - __iscsit_free_cmd(cmd, shutdown);
> - target_put_sess_cmd(se_cmd);
> - }
> - break;
> - }
> - /* Fall-through */
> - default:
> - __iscsit_free_cmd(cmd, shutdown);
> + } else {
> iscsit_release_cmd(cmd);
> - break;
> }
> }
> EXPORT_SYMBOL(iscsit_free_cmd);
>
Maybe it's easier just to add
if (!se_cmd) {
iscsit_release_cmd(cmd);
return;
}
to the beginning of the function; that would reduce code churn by quite
a bit ...
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
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
