On 6/14/16, 1:13 AM, "Nicholas A. Bellinger" <target-devel-owner@xxxxxxxxxxxxxxx on behalf of nab@xxxxxxxxxxxxxxx> wrote:
<SNIP>
>>
>> >
>> > if (sense_reason != TCM_UNSUPPORTED_SCSI_OPCODE ||
>> > sense_reason != TCM_INVALID_CDB_FIELD)
>> > transport_complete_task_attr(cmd);
>>
>> I don't think this works for a fix for the case I mentioned
>> (transport_check_alloc_task_attr failing). TCM_INVALID_CDB_FIELD can
>> get generated for a whole lot of cases, for most of which we want to
>> call transport_complete_task_attr.
>>
>> It seems like this might be more of a design flaw to which we're just
>> trying to apply band-aids. I'm not sure what the right fix is, but I
>> don't think this is it.
>
>Ok, how about something along the lines of the following..?
>
>Namely, to set a flag only once we've actually reached
>target_handle_task_attr(), otherwise in transport_complete_task_attr()
>avoid messing with se_device task attribute counters.
>
>So the assumption is until we reach target_execute_cmd() ->
>target_handle_task_attr(), all exceptions will not be touching se_device
>task attribute counters.
>
>WDYT..?
>
>diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
>index bd77467..21dd65a 100644
>--- a/drivers/target/target_core_transport.c
>+++ b/drivers/target/target_core_transport.c
>@@ -1827,6 +1827,8 @@ static bool target_handle_task_attr(struct se_cmd *cmd)
> if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
> return false;
>
>+ cmd->se_cmd_flags |= SCF_TASK_ATTR_SET;
>+
> /*
> * Check for the existence of HEAD_OF_QUEUE, and if true return 1
> * to allow the passed struct se_cmd list of tasks to the front of the list.
>@@ -1949,6 +1951,9 @@ static void transport_complete_task_attr(struct se_cmd *cmd)
> if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
> return;
>
>+ if (!(cmd->se_cmd_flags & SCF_TASK_ATTR_SET))
>+ goto restart;
>+
> if (cmd->sam_task_attr == TCM_SIMPLE_TAG) {
> atomic_dec_mb(&dev->simple_cmds);
> dev->dev_cur_ordered_id++;
>@@ -1965,7 +1970,7 @@ static void transport_complete_task_attr(struct se_cmd *cmd)
> pr_debug("Incremented dev_cur_ordered_id: %u for ORDERED\n",
> dev->dev_cur_ordered_id);
> }
>-
>+restart:
> target_restart_delayed_cmds(dev);
> }
>
>diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
>index b316b44..fb8e3b6 100644
>--- a/include/target/target_core_base.h
>+++ b/include/target/target_core_base.h
>@@ -142,6 +142,7 @@ enum se_cmd_flags_table {
> SCF_PASSTHROUGH_PROT_SG_TO_MEM_NOALLOC = 0x00200000,
> SCF_ACK_KREF = 0x00400000,
> SCF_USE_CPUID = 0x00800000,
>+ SCF_TASK_ATTR_SET = 0x01000000,
> };
>
> /*
>--
>1.9.1
>
>--
Looks good and works well for me, you can add a Tested-by:, Reviewed-by: Bryant G. Ly <bryantly@xxxxxxxxxxxxxxxxxx>
-Bryant
--
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
