Re: [PATCH 4/4] ufs: Let the SCSI core allocate per-command UFS data

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

On 1/9/20 1:48 AM, Avri Altman wrote:
> Bart Van Assche wrote:
>> +       /* See also ufshcd_is_scsi() */
>> +       switch (req_op(cmd->request)) {
>> +       case REQ_OP_DRV_IN:
>> +       case REQ_OP_DRV_OUT:
>> +               WARN_ON_ONCE(true);
 >
> Maybe just WARN_ON_ONCE(!ufshcd_is_scsi(cmd->request))

Good idea. Will do.

>> +static int ufshcd_init_cmd_priv(struct Scsi_Host *shost, struct
>> +scsi_cmnd *cmd) {
>> +       struct ufs_hba *hba = shost_priv(shost);
>> +
>> +       ufshcd_init_lrb(hba, scsi_cmd_priv(cmd), cmd->tag);
 >
> So ufshcd_init_lrb() is called now for every new request?

ufshcd_init_lrb() is only called from inside scsi_add_host(), namely as 
follows:

scsi_add_host()
-> scsi_add_host_with_dma()
   -> scsi_mq_setup_tags()
     -> blk_mq_alloc_tag_set()
       -> blk_mq_alloc_rq_maps()
         -> __blk_mq_alloc_rq_maps()
           -> __blk_mq_alloc_rq_map()
             -> blk_mq_alloc_rqs()
               -> blk_mq_init_request()
                 -> scsi_mq_init_request()
                   -> ufshcd_init_cmd_priv()

>> @@ -6074,7 +6132,8 @@ static int ufshcd_eh_device_reset_handler(struct
>> scsi_cmnd *cmd)
>>
>>          /* clear the commands that were pending for corresponding LUN */
>>          for_each_set_bit(pos, &hba->outstanding_reqs, hba->nutrs) {
>> -               if (hba->lrb[pos].lun == lrbp->lun) {
>> +               lrbp2 = ufshcd_tag_to_lrb(hba, pos);
 >
> Can lrpb2 be null here?

lrpb2 can only be NULL if the 'pos' argument passed to 
ufshcd_tag_to_lrb() is not a valid tag. for_each_set_bit() however 
guarantees that 0 <= pos < hba->nutrs and hence guarantees that 'pos' is 
a valid tag.

Thanks,

Bart.