On Tue, 2015-04-14 at 13:26 +0200, Bart Van Assche wrote:
> The loop in core_tmr_abort_task() iterates over sess_cmd_list.
> That list is a list of regular commands and task management
> functions (TMFs). Skip TMFs in this loop instead of letting
> the target drivers filter out TMFs in their get_task_tag()
> callback function.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Andy Grover <agrover@xxxxxxxxxx>
> Cc: <qla2xxx-upstream@xxxxxxxxxx>
> ---
> drivers/scsi/qla2xxx/tcm_qla2xxx.c | 4 ----
> drivers/target/target_core_tmr.c | 4 ++--
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> index 9056b52..d5c4982 100644
> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> @@ -537,10 +537,6 @@ static u32 tcm_qla2xxx_get_task_tag(struct se_cmd *se_cmd)
> {
> struct qla_tgt_cmd *cmd;
>
> - /* check for task mgmt cmd */
> - if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
> - return 0xffffffff;
> -
> cmd = container_of(se_cmd, struct qla_tgt_cmd, se_cmd);
>
> return cmd->tag;
Sigh. Really, don't drop checks that where added for a specific reason,
without understanding why they where added in the first place.
Here, it's totally wrong to do a container_of() for TMRs, and the Qlogic
folks added this specific check in commit dd9c4eff to address this bug.
Changing core_tmr_abort_task() below addresses one case, but it's not
the only case where ->get_task_tag() can be called for a TMR.
> diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
> index fa5e157..315ec34 100644
> --- a/drivers/target/target_core_tmr.c
> +++ b/drivers/target/target_core_tmr.c
> @@ -125,8 +125,8 @@ void core_tmr_abort_task(
> if (dev != se_cmd->se_dev)
> continue;
>
> - /* skip se_cmd associated with tmr */
> - if (tmr->task_cmd == se_cmd)
> + /* skip task management functions, including tmr->task_cmd */
> + if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
> continue;
>
> ref_tag = se_cmd->se_tfo->get_task_tag(se_cmd);
This part is fine.
Applied to target-pending/for-next, without the bogus
tcm_qla2xxx_get_task_tag() check removal.
--nab
--
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
