OK. Thanks.
Please add my Reviewed-by tag to your next ver.
Thanks,
Avri
>
>
> 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.
