RE: RE: [PATCH v22 4/4] scsi: ufs: Add HPB 2.0 support
From: Daejun Park <hidden>
Date: 2021-02-24 00:45:05
Also in:
lkml
quoted
@@ -2656,7 +2656,12 @@ static int ufshcd_queuecommand(struct Scsi_Host*host, struct scsi_cmnd *cmd) lrbp->req_abort_skip = false; - ufshpb_prep(hba, lrbp); + err = ufshpb_prep(hba, lrbp); + if (err == -EAGAIN) { + lrbp->cmd = NULL; + ufshcd_release(hba); + goto out; + }Did I miss-read it, or are you bailing out of wb failed e.g. because no tag is available? Why not continue with read10?
We try to sending HPB read several times within the requeue_timeout_ms. Because it strategy has more benefit for overall performance in this situation that many requests are queueing.
quoted
+ if (blk_insert_cloned_request(q, req) != BLK_STS_OK) + return -EAGAIN;Why did you choose to use blk_insert_cloned_request and not e.g. the more common blk_execute_rq_nowait?
It is the process that sending one more command (write buffer) prior to HPB read command. This API makes write buffer to issue directly. Other APIs, for example blk_execute_rq_nowait, it can make queueing the command in the scheduler, so the order of commands can be inversed. Here is comment of the API. // blk_insert_cloned_request - Helper for stacking drivers to submit a request
quoted
+ hpb->stats.pre_req_cnt++; + + return 0; +}quoted
- ufshpb_set_hpb_read_to_upiu(hpb, lrbp, lpn, ppn, transfer_len); + if (ufshpb_is_required_wb(hpb, transfer_len)) { + err = ufshpb_issue_pre_req(hpb, cmd, &read_id); + if (err) { + unsigned long timeout; + + timeout = cmd->jiffies_at_alloc + msecs_to_jiffies( + hpb->params.requeue_timeout_ms); + if (time_before(jiffies, timeout)) + return -EAGAIN;Why requeue_timeout_ms needs to be a configurable parameter? Why rq->timeout is not enough?
We are using this value for re-trying threshold of HPB read. Thanks, Daejun
Thanks, Avri