RE: [PATCH v4] scsi: ufs: core: handle PM commands timeout before SCSI EH

[Fang Hongjie(方洪杰)]

> 
> I know, but what I mean is, if this check (!hba->mcq_enabled)
> always evaluates to true, then it is a redundant check, right?
> I don't see any scenario where it would return false.
> 

!hba->mcq_enabled is not always true once PM internal
device-management commands are included. In MCQ mode,
ufshcd_mcq_force_compl_one() skips reserved requests, so after
ufshcd_link_recovery() a timed-out reserved internal command can still
reach this path with hba->mcq_enabled == true. 

> >
> > >
> > > > +                       unsigned long flags;
> > > > +                       struct request *rq =
> > > > scsi_cmd_to_rq(scmd);
> > > > +
> > > > +                       spin_lock_irqsave(&hba->outstanding_lock,
> > > > flags);
> > > > +                       __clear_bit(rq->tag, &hba-
> > > > >outstanding_reqs);
> > > > +                       spin_unlock_irqrestore(&hba-
> > > > > outstanding_lock, flags);
> > > > +               }
> > > > +
> > > > +               set_host_byte(scmd, DID_TIME_OUT);
> > > >
> > >
> > > Why does MCQ mode set DID_REQUEUE, while legacy mode sets
> > > DID_TIME_OUT?
> > >
> > Normal SCSI PM commands use DID_REQUEUE, matching the existing
> > MCQ force-completion behavior. Reserved internal device-management
> > commands continue to use DID_TIME_OUT, right ?
> >
> 
> Sorry, I don't get it.
> I mean, in MCQ mode, the ufshcd_mcq_force_compl_one function
> handles the same uncompleted condition, but why is the processing
> logic different?
> 
> if (!test_bit(SCMD_STATE_COMPLETE, &cmd->state)) {
>     set_host_byte(cmd, DID_REQUEUE);
>     ufshcd_release_scsi_cmd(hba, cmd);
>     scsi_done(cmd);

Plan to use DID_REQUEUE for normal SCSI commands, since this matches
the existing MCQ force-completion behavior.

For reserved internal commands, these result are returned directly to the
UFS core caller rather than going through the normal scsi_execute_cmd()
retry flow. Therefore, I plan to continue using DID_TIME_OUT for those
commands.



Best.