On 12/31/20 10:45 PM, Bart Van Assche wrote:
> On 10/25/20 4:03 PM, Mike Christie wrote:
>> @@ -617,25 +629,20 @@ static int tcm_qla2xxx_handle_tmr(struct qla_tgt_mgmt_cmd *mcmd, u64 lun,
>> static struct qla_tgt_cmd *tcm_qla2xxx_find_cmd_by_tag(struct fc_port *sess,
>> uint64_t tag)
>> {
>> - struct qla_tgt_cmd *cmd = NULL;
>> - struct se_cmd *secmd;
>> + struct qla_tgt_cmd *cmd;
>> unsigned long flags;
>>
>> if (!sess->se_sess)
>> return NULL;
>>
>> - spin_lock_irqsave(&sess->se_sess->sess_cmd_lock, flags);
>> - list_for_each_entry(secmd, &sess->se_sess->sess_cmd_list, se_cmd_list) {
>> - /* skip task management functions, including tmr->task_cmd */
>> - if (secmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
>> - continue;
>> -
>> - if (secmd->tag == tag) {
>> - cmd = container_of(secmd, struct qla_tgt_cmd, se_cmd);
>> - break;
>> - }
>> + spin_lock_irqsave(&sess->sess_cmd_lock, flags);
>> + list_for_each_entry(cmd, &sess->sess_cmd_list, sess_cmd_list) {
>> + if (cmd->se_cmd.tag == tag)
>> + goto done;
>> }
>> - spin_unlock_irqrestore(&sess->se_sess->sess_cmd_lock, flags);
>> + cmd = NULL;
>> +done:
>> + spin_unlock_irqrestore(&sess->sess_cmd_lock, flags);
>>
>> return cmd;
>> }
>
> Hi Mike,
>
> Although this behavior has not been introduced by your patch: what prevents
> that the command found by tcm_qla2xxx_find_cmd_by_tag() disappears after
> sess_cmd_lock has been unlocked and before the caller uses the qla_tgt_cmd
> pointer? As you may know the corresponding code in SCST increments the SCSI
Nothing.
> command reference count before unlocking the lock that protects the command
> list. See also the __scst_find_cmd_by_tag() call in scst_mgmt_cmd_init().
>
I'll send a patch for that when I get the aborted task crash fixed up. I
didn't send fixes for existing bugs in the driver like them for this patchset.
It got a little crazy. For example for the aborted task issue, I reverted the
patch that made the eh async I mentioned a while back. That fixes the crash,
but then there was a hang. So I thought I'll just convert it to the async eh
patch since either way I have to fix the driver. Himanshu was helping me figure
out how to support it, but it's not trivial.
