On 2021-05-14 12:04, Can Guo wrote:
Hi Bart,
On 2021-05-14 00:49, Bart Van Assche wrote:
With the current implementation of the UFS driver active_queues is 1
instead of 0 if all UFS request queues are idle. That causes
hctx_may_queue() to divide the queue depth by 2 when queueing a
request
and hence reduces the usable queue depth.
This is interesting. When all UFS queues are idle, in hctx_may_queue(),
active_queues reads 1 (users == 1, depth == 32), where is it divided by
2?
Are you saying that if we queue a new request on one of the UFS request
queues, since the active_queues is always above 0, we can never use
the full queue depth? If so, then I agree.
Reviewed-by: Can Guo <cang@xxxxxxxxxxxxxx>
static inline bool hctx_may_queue(struct blk_mq_hw_ctx *hctx,
struct sbitmap_queue *bt)
{
unsigned int depth, users;
....
users = atomic_read(&hctx->tags->active_queues);
}
if (!users)
return true;
/*
* Allow at least some tags
*/
depth = max((bt->sb.depth + users - 1) / users, 4U);
return __blk_mq_active_requests(hctx) < depth;
}
Thanks,
Can Guo.
The shared tag set code in the block layer keeps track of the number
of
active request queues. blk_mq_tag_busy() is called before a request is
queued onto a hwq and blk_mq_tag_idle() is called some time after the
hwq
became idle. blk_mq_tag_idle() is called from inside
blk_mq_timeout_work().
Hence, blk_mq_tag_idle() is only called if a timer is associated with
each
request that is submitted to a request queue that shares a tag set
with
another request queue. Hence this patch that adds a
blk_mq_start_request()
call in ufshcd_exec_dev_cmd(). This patch doubles the queue depth on
my
test setup from 16 to 32.
In addition to increasing the usable queue depth, also fix the
documentation of the 'timeout' parameter in the header above
ufshcd_exec_dev_cmd().
Cc: Can Guo <cang@xxxxxxxxxxxxxx>
Cc: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>
Cc: Avri Altman <avri.altman@xxxxxxx>
Cc: Stanley Chu <stanley.chu@xxxxxxxxxxxx>
Cc: Bean Huo <beanhuo@xxxxxxxxxx>
Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Fixes: 7252a3603015 ("scsi: ufs: Avoid busy-waiting by eliminating tag
conflicts")
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
---
drivers/scsi/ufs/ufshcd.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c96e36aab989..e669243354da 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2838,7 +2838,7 @@ static int ufshcd_wait_for_dev_cmd(struct
ufs_hba *hba,
* ufshcd_exec_dev_cmd - API for sending device management requests
* @hba: UFS hba
* @cmd_type: specifies the type (NOP, Query...)
- * @timeout: time in seconds
+ * @timeout: timeout in milliseconds
*
* NOTE: Since there is only one available tag for device management
commands,
* it is expected you hold the hba->dev_cmd.lock mutex.
@@ -2868,6 +2868,9 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba
*hba,
}
tag = req->tag;
WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));
+ /* Set the timeout such that the SCSI error handler is not
activated. */
+ req->timeout = msecs_to_jiffies(2 * timeout);
+ blk_mq_start_request(req);
init_completion(&wait);
lrbp = &hba->lrb[tag];
