* [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests
@ 2023-03-31 7:46 Avri Altman
2023-03-31 8:04 ` Johannes Thumshirn
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Avri Altman @ 2023-03-31 7:46 UTC (permalink / raw)
To: James E . J . Bottomley, Martin K . Petersen
Cc: Asutosh Das, quic_cang, linux-scsi, linux-kernel,
Johannes Thumshirn, Bart Van Assche, Avri Altman
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.
---
v1 -> v2:
Attend Johannes's and Bart's comments
Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
drivers/ufs/core/ufshcd.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 35a3bd95c5e4..cac7c9918c5b 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8468,6 +8468,11 @@ 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");
+ 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.
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests
2023-03-31 7:46 [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests Avri Altman
@ 2023-03-31 8:04 ` Johannes Thumshirn
2023-04-03 6:11 ` Dan Carpenter
2023-04-03 8:52 ` John Garry
2 siblings, 0 replies; 6+ messages in thread
From: Johannes Thumshirn @ 2023-03-31 8:04 UTC (permalink / raw)
To: Avri Altman, James E . J . Bottomley, Martin K . Petersen
Cc: Asutosh Das, quic_cang, linux-scsi, linux-kernel, Bart Van Assche
Looks good code wise, though I have 0 clue of UFS.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests
2023-03-31 7:46 [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests Avri Altman
2023-03-31 8:04 ` Johannes Thumshirn
@ 2023-04-03 6:11 ` Dan Carpenter
2023-04-20 9:59 ` Avri Altman
2023-04-03 8:52 ` John Garry
2 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2023-04-03 6:11 UTC (permalink / raw)
To: oe-kbuild, Avri Altman, James E . J . Bottomley, Martin K . Petersen
Cc: lkp, oe-kbuild-all, Asutosh Das, quic_cang, linux-scsi,
linux-kernel, Johannes Thumshirn, Bart Van Assche, Avri Altman
Hi Avri,
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Avri-Altman/scsi-ufs-mcq-Limit-the-amount-of-inflight-requests/20230331-155149
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link: https://lore.kernel.org/r/20230331074650.75-1-avri.altman%40wdc.com
patch subject: [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests
config: parisc-randconfig-m031-20230329 (https://download.01.org/0day-ci/archive/20230401/202304011340.ltlHYazS-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202304011340.ltlHYazS-lkp@intel.com/
New smatch warnings:
drivers/ufs/core/ufshcd.c:8473 ufshcd_alloc_mcq() warn: missing error code 'ret'
Old smatch warnings:
drivers/ufs/core/ufshcd.c:5412 ufshcd_uic_cmd_compl() error: we previously assumed 'hba->active_uic_cmd' could be null (see line 5400)
drivers/ufs/core/ufshcd.c:2350 ufshcd_hba_capabilities() warn: missing error code? 'err'
vim +/ret +8473 drivers/ufs/core/ufshcd.c
57b1c0ef89ac9d drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8457 static int ufshcd_alloc_mcq(struct ufs_hba *hba)
57b1c0ef89ac9d drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8458 {
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8459 int ret;
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8460 int old_nutrs = hba->nutrs;
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8461
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8462 ret = ufshcd_mcq_decide_queue_depth(hba);
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8463 if (ret < 0)
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8464 return ret;
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8465
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8466 hba->nutrs = ret;
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8467 ret = ufshcd_mcq_init(hba);
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8468 if (ret)
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8469 goto err;
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8470
2580a95e61d461 drivers/ufs/core/ufshcd.c Avri Altman 2023-03-31 8471 if (hba->nutrs * hba->nr_hw_queues > SZ_64K - 1) {
2580a95e61d461 drivers/ufs/core/ufshcd.c Avri Altman 2023-03-31 8472 dev_info(hba->dev, "there can be at most 64K inflight requests\n");
2580a95e61d461 drivers/ufs/core/ufshcd.c Avri Altman 2023-03-31 @8473 goto err;
ret = -EINVAL;
2580a95e61d461 drivers/ufs/core/ufshcd.c Avri Altman 2023-03-31 8474 }
2580a95e61d461 drivers/ufs/core/ufshcd.c Avri Altman 2023-03-31 8475
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8476 /*
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8477 * Previously allocated memory for nutrs may not be enough in MCQ mode.
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8478 * Number of supported tags in MCQ mode may be larger than SDB mode.
6ccf44fe4cd7c4 drivers/scsi/ufs/ufshcd.c Seungwon Jeon 2013-06-26 8479 */
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8480 if (hba->nutrs != old_nutrs) {
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8481 ufshcd_release_sdb_queue(hba, old_nutrs);
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8482 ret = ufshcd_memory_alloc(hba);
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8483 if (ret)
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8484 goto err;
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8485 ufshcd_host_memory_configure(hba);
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8486 }
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8487
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8488 ret = ufshcd_mcq_memory_alloc(hba);
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8489 if (ret)
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8490 goto err;
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8491
7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8492 return 0;
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8493 err:
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8494 hba->nutrs = old_nutrs;
4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8495 return ret;
57b1c0ef89ac9d drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8496 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests
2023-04-03 6:11 ` Dan Carpenter
@ 2023-04-20 9:59 ` Avri Altman
0 siblings, 0 replies; 6+ messages in thread
From: Avri Altman @ 2023-04-20 9:59 UTC (permalink / raw)
To: Dan Carpenter, oe-kbuild, James E . J . Bottomley, Martin K . Petersen
Cc: lkp, oe-kbuild-all, Asutosh Das, quic_cang, linux-scsi,
linux-kernel, Johannes Thumshirn, Bart Van Assche
> Hi Avri,
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Avri-Altman/scsi-ufs-
> mcq-Limit-the-amount-of-inflight-requests/20230331-155149
> base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
> patch link: https://lore.kernel.org/r/20230331074650.75-1-
> avri.altman%40wdc.com
> patch subject: [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests
> config: parisc-randconfig-m031-20230329 (https://download.01.org/0day-
> ci/archive/20230401/202304011340.ltlHYazS-lkp@intel.com/config)
> compiler: hppa-linux-gcc (GCC) 12.1.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Link: https://lore.kernel.org/r/202304011340.ltlHYazS-lkp@intel.com/
>
> New smatch warnings:
> drivers/ufs/core/ufshcd.c:8473 ufshcd_alloc_mcq() warn: missing error code
> 'ret'
>
> Old smatch warnings:
> drivers/ufs/core/ufshcd.c:5412 ufshcd_uic_cmd_compl() error: we previously
> assumed 'hba->active_uic_cmd' could be null (see line 5400)
> drivers/ufs/core/ufshcd.c:2350 ufshcd_hba_capabilities() warn: missing error
> code? 'err'
>
> vim +/ret +8473 drivers/ufs/core/ufshcd.c
>
> 57b1c0ef89ac9d drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8457
> static int ufshcd_alloc_mcq(struct ufs_hba *hba)
> 57b1c0ef89ac9d drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8458 {
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8459
> int ret;
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8460
> int old_nutrs = hba->nutrs;
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8461
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8462
> ret = ufshcd_mcq_decide_queue_depth(hba);
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8463
> if (ret < 0)
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8464
> return ret;
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8465
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8466
> hba->nutrs = ret;
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8467
> ret = ufshcd_mcq_init(hba);
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8468
> if (ret)
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8469
> goto err;
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8470
> 2580a95e61d461 drivers/ufs/core/ufshcd.c Avri Altman 2023-03-31 8471
> if (hba->nutrs * hba->nr_hw_queues > SZ_64K - 1) {
> 2580a95e61d461 drivers/ufs/core/ufshcd.c Avri Altman 2023-03-31 8472
> dev_info(hba->dev, "there can be at most 64K inflight requests\n");
> 2580a95e61d461 drivers/ufs/core/ufshcd.c Avri Altman 2023-03-31 @8473
> goto err;
>
> ret = -EINVAL;
Thanks.
Avri
>
> 2580a95e61d461 drivers/ufs/core/ufshcd.c Avri Altman 2023-03-31 8474
> }
> 2580a95e61d461 drivers/ufs/core/ufshcd.c Avri Altman 2023-03-31 8475
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8476
> /*
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8477
> * Previously allocated memory for nutrs may not be enough in MCQ mode.
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8478
> * Number of supported tags in MCQ mode may be larger than SDB mode.
> 6ccf44fe4cd7c4 drivers/scsi/ufs/ufshcd.c Seungwon Jeon 2013-06-26 8479
> */
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8480
> if (hba->nutrs != old_nutrs) {
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8481
> ufshcd_release_sdb_queue(hba, old_nutrs);
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8482
> ret = ufshcd_memory_alloc(hba);
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8483
> if (ret)
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8484
> goto err;
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8485
> ufshcd_host_memory_configure(hba);
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8486
> }
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8487
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8488
> ret = ufshcd_mcq_memory_alloc(hba);
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8489
> if (ret)
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8490
> goto err;
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8491
> 7224c806876e46 drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8492
> return 0;
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8493
> err:
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8494
> hba->nutrs = old_nutrs;
> 4682abfae2eb3a drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8495
> return ret;
> 57b1c0ef89ac9d drivers/ufs/core/ufshcd.c Asutosh Das 2023-01-13 8496 }
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests
2023-03-31 7:46 [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests Avri Altman
2023-03-31 8:04 ` Johannes Thumshirn
2023-04-03 6:11 ` Dan Carpenter
@ 2023-04-03 8:52 ` John Garry
2023-04-20 10:06 ` Avri Altman
2 siblings, 1 reply; 6+ messages in thread
From: John Garry @ 2023-04-03 8:52 UTC (permalink / raw)
To: Avri Altman, James E . J . Bottomley, Martin K . Petersen
Cc: Asutosh Das, quic_cang, linux-scsi, linux-kernel,
Johannes Thumshirn, Bart Van Assche
On 31/03/2023 08:46, 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.
>
> ---
> v1 -> v2:
> Attend Johannes's and Bart's comments
>
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
> drivers/ufs/core/ufshcd.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 35a3bd95c5e4..cac7c9918c5b 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8468,6 +8468,11 @@ static int ufshcd_alloc_mcq(struct ufs_hba *hba)
> if (ret)
> goto err;
>
> + if (hba->nutrs * hba->nr_hw_queues > SZ_64K - 1) {
If shost->host_tagset is set - which it seems to be for this driver -
then the number of HW queues would not influence how many IOs the host
may be sent. Rather this is just limited by just the HW queue depth.
Thanks,
John
> + dev_info(hba->dev, "there can be at most 64K inflight requests\n");
> + 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.
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests
2023-04-03 8:52 ` John Garry
@ 2023-04-20 10:06 ` Avri Altman
0 siblings, 0 replies; 6+ messages in thread
From: Avri Altman @ 2023-04-20 10:06 UTC (permalink / raw)
To: John Garry, James E . J . Bottomley, Martin K . Petersen
Cc: Asutosh Das, quic_cang, linux-scsi, linux-kernel,
Johannes Thumshirn, Bart Van Assche
> On 31/03/2023 08:46, 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.
> >
> > ---
> > v1 -> v2:
> > Attend Johannes's and Bart's comments
> >
> > Signed-off-by: Avri Altman <avri.altman@wdc.com>
> > ---
> > drivers/ufs/core/ufshcd.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index 35a3bd95c5e4..cac7c9918c5b 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -8468,6 +8468,11 @@ static int ufshcd_alloc_mcq(struct ufs_hba *hba)
> > if (ret)
> > goto err;
> >
> > + if (hba->nutrs * hba->nr_hw_queues > SZ_64K - 1) {
>
> If shost->host_tagset is set - which it seems to be for this driver - then the
> number of HW queues would not influence how many IOs the host may be
> sent. Rather this is just limited by just the HW queue depth.
Thanks.
The purpose of this patch is merely to document the ufs spec restrictions.
practically, it impose no functional change.
I will elaborate the commit log accordingly.
Thanks,
Avri
>
> Thanks,
> John
>
> > + dev_info(hba->dev, "there can be at most 64K inflight requests\n");
> > + 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-04-20 10:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-31 7:46 [PATCH v2] scsi: ufs: mcq: Limit the amount of inflight requests Avri Altman
2023-03-31 8:04 ` Johannes Thumshirn
2023-04-03 6:11 ` Dan Carpenter
2023-04-20 9:59 ` Avri Altman
2023-04-03 8:52 ` John Garry
2023-04-20 10:06 ` Avri Altman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®