Re: [PATCH 12/24] scsi: ufs: core: Rework ufshcd_mcq_compl_pending_transfer()

[Bart Van Assche <bvanassche@acm.org>]

On 4/15/25 1:00 AM, Peter Wang (王信友) wrote:
>>   /**
>>    * ufshcd_mcq_compl_pending_transfer - MCQ mode function. It is
>>    * invoked from the error handler context or
>> ufshcd_host_reset_and_restore()
>> @@ -5665,38 +5703,10 @@ static int ufshcd_poll(struct Scsi_Host
>> *shost, unsigned int queue_num)
>>   static void ufshcd_mcq_compl_pending_transfer(struct ufs_hba *hba,
>>                                                bool force_compl)
>>   {
>> -       struct ufs_hw_queue *hwq;
>> -       struct ufshcd_lrb *lrbp;
>> -       struct scsi_cmnd *cmd;
>> -       unsigned long flags;
>> -       int tag;
>> -
>> -       for (tag = 0; tag < hba->nutrs; tag++) {
>> -               lrbp = &hba->lrb[tag];
>> -               cmd = lrbp->cmd;
>> -               if (!ufshcd_cmd_inflight(cmd) ||
>> -                   test_bit(SCMD_STATE_COMPLETE, &cmd->state))
>> -                       continue;
>>
> 
> Removing this check might cause racing issues?
> Leading to the possibility that the hwq in the subsequent function
> could be null?

Hi Peter,

The ufshcd_cmd_inflight() check has not been removed.
blk_mq_tagset_busy_iter() only calls the callback function that is 
passed as second argument for requests that have been started. The
definition of ufshcd_cmd_inflight() is as follows:

bool ufshcd_cmd_inflight(struct scsi_cmnd *cmd)
{
	return cmd &&
	       blk_mq_rq_state(scsi_cmd_to_rq(cmd)) == MQ_RQ_IN_FLIGHT;
}

 From the blk_mq_tagset_busy_iter() implementation:

	if (!(iter_data->flags & BT_TAG_ITER_STARTED) ||
	    blk_mq_request_started(rq))
		ret = iter_data->fn(rq, iter_data->data);

 From blk-mq.h:

static inline int blk_mq_request_started(struct request *rq)
{
	return blk_mq_rq_state(rq) != MQ_RQ_IDLE;
}

In other words, if flag BT_TAG_ITER_STARTED has not been set,
blk_mq_tagset_busy_iter() only calls its callback function for requests
for which blk_mq_start_request() has been called and that have not yet
been freed.

Bart.