RE: [PATCH v3] scsi: ufs: mcq: Limit the amount of inflight requests

[Avri Altman]

> On 20/04/2023 11:11, Avri Altman wrote:
> > in UFS, each request is designated via the triplet <iid, lun, task tag>.
> >
> > In UFS4.0 the Initiator ID field is 8 bits wide, comprised of the
> > EXT_IID and IID fields. Together with the task tag (single byte), they
> > limit the driver's hw queues capacity.
> >
> > The purpose of this patch is to document the ufs spec restrictions.
> > practically, it impose no functional change because a) the current hw
> > supports much less (merely 64 requests), and b) since
> > shost->host_tagset is set - then the number of inflight IOs is limited
> > by just the HW queue depth.
> >
> 
> I don't know why effectively it has no change, as it seems to me to be checking
> and failing something valid which could happen.
> 
> If it really cannot happen, then we generally don't add what is effectively
> dead code just to provide practical illustrations of the spec. And it is
> misleading to others who may use your driver as a reference.
Better to just drop it then.

Thanks,
Avri

> 
> A code comment would be a better choice, if anything, IMHO.
> 
> Thanks,
> John
> 
> > ---
> > v2 -> v3:
> > fix smatch warnings (Dan Carpenter)
> > elaborate commit log (John Garry)
> >
> > v1 -> v2:
> > Attend Johannes's and Bart's comments
> 
> This is the wrong place for these
> 
> >
> > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
> > Signed-off-by: Avri Altman <avri.altman@xxxxxxx>
> > ---
> >   drivers/ufs/core/ufshcd.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index 03c47f9a2750..9cebec6be35c 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -8468,6 +8468,12 @@ static int ufshcd_alloc_mcq(struct ufs_hba *hba)
> >       if (ret)
> >               goto err;
> >
> > +     if (hba->nutrs * hba->nr_hw_queues > SZ_64K - 1) {
> > +             dev_info(hba->dev, "there can be at most 64K inflight requests\n");
> > +             ret = -EINVAL;
> > +             goto err;
> > +     }
> > +
> >       /*
> >        * Previously allocated memory for nutrs may not be enough in MCQ
> mode.
> >        * Number of supported tags in MCQ mode may be larger than SDB
> mode.