On 25.04.20 20:07, Bart Van Assche wrote:
On 2020-04-16 13:31, huobean@xxxxxxxxx wrote:
+static int ufshpb_execute_cmd(struct ufshpb_lu *hpb, unsigned char *cmd)
+{
+ struct scsi_sense_hdr sshdr;
+ struct scsi_device *sdp;
+ struct ufs_hba *hba;
+ int retries;
+ int ret = 0;
+
+ hba = hpb->hba;
+
+ sdp = hba->sdev_ufs_lu[hpb->lun];
+ if (!sdp) {
+ hpb_warn("%s UFSHPB cannot find scsi_device\n", __func__);
+ return -ENODEV;
+ }
+
+ ret = scsi_device_get(sdp);
+ if (!ret && !scsi_device_online(sdp)) {
+ ret = -ENODEV;
+ scsi_device_put(sdp);
+ return ret;
+ }
+
+ for (retries = 3; retries > 0; --retries) {
+ ret = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr,
+ msecs_to_jiffies(30000), 3, 0, RQF_PM, NULL);
+ if (ret == 0)
+ break;
+ }
+
+ if (ret) {
+ if (driver_byte(ret) == DRIVER_SENSE &&
+ scsi_sense_valid(&sshdr)) {
+ switch (sshdr.sense_key) {
+ case ILLEGAL_REQUEST:
+ hpb_err("ILLEGAL REQUEST asc=0x%x ascq=0x%x\n",
+ sshdr.asc, sshdr.ascq);
+ break;
+ default:
+ hpb_err("Unknown return code = %x\n", ret);
+ break;
+ }
+ }
+ }
+ scsi_device_put(sdp);
+
+ return ret;
+}
If scsi_execute() would be changed into asynchronous SCSI command
submission, can ufshpb_execute_cmd() be called from inside the UFS
.queue_rq() callback instead of from workqueue context?
The scsi_device_get() call looks misplaced. I think that call should
happen before schedule_work() is called.
+static int ufshpb_l2p_load_req(struct ufshpb_lu *hpb, struct request_queue *q,
+ struct ufshpb_map_req *map_req)
+{
+ struct scsi_request *scsi_rq;
+ unsigned char cmd[16] = { };
+ struct request *req;
+ struct bio *bio;
+ int alloc_len;
+ int ret;
+
+ bio = &map_req->bio;
+
+ ret = ufshpb_add_bio_page(hpb, q, bio, map_req->bvec, map_req->mctx);
+ if (ret) {
+ hpb_err("ufshpb_add_bio_page() failed with ret %d\n", ret);
+ return ret;
+ }
+
+ alloc_len = hpb->hba->hpb_geo.subregion_entry_sz;
+ /*
+ * HPB Sub-Regions are equally sized except for the last one which is
+ * smaller if the last hpb Region is not an integer multiple of
+ * bHPBSubRegionSize.
+ */
+ if (map_req->region == (hpb->lu_region_cnt - 1) &&
+ map_req->subregion == (hpb->subregions_in_last_region - 1))
+ alloc_len = hpb->last_subregion_entry_size;
+
+ ufshpb_prepare_read_buf_cmd(cmd, map_req->region, map_req->subregion,
+ alloc_len);
+ if (!map_req->req) {
+ map_req->req = blk_get_request(q, REQ_OP_SCSI_IN, 0);
+ if (IS_ERR(map_req->req))
+ return PTR_ERR(map_req->req);
+ }
+ req = map_req->req;
+ scsi_rq = scsi_req(req);
+
+ blk_rq_append_bio(req, &bio);
+
+ scsi_req_init(scsi_rq);
+
+ scsi_rq->cmd_len = COMMAND_SIZE(cmd[0]);
+ memcpy(scsi_rq->cmd, cmd, scsi_rq->cmd_len);
+ req->timeout = msecs_to_jiffies(30000);
+ req->end_io_data = (void *)map_req;
+
+ hpb_dbg(SCHEDULE_INFO, hpb, "ISSUE READ_BUFFER : (%d-%d) retry = %d\n",
+ map_req->region, map_req->subregion, map_req->retry_cnt);
+ hpb_trace(hpb, "%s: I RB %d - %d", DRIVER_NAME, map_req->region,
+ map_req->subregion);
+
+ blk_execute_rq_nowait(q, NULL, req, 1, ufshpb_l2p_load_req_done);
+ map_req->req_issue_t = ktime_to_ns(ktime_get());
+
+ atomic64_inc(&hpb->status.map_req_cnt);
+
+ return 0;
+}
Same question here: if 'req' would be submitted asynchronously, can
ufshpb_l2p_load_req() be called directly from inside the UFS .queue_rq()
callback instead of from workqueue context?
Thanks,
Bart.
Hi, Bart
Thanks very much.
If I understood the Christoph's the second question correctly. Enqueue
HPB requests to the
scsi_device->request_queueu, and fly back to SCSI schedule, it's
unacceptable. I don't like this
implementation way either. Also, the latency of the L2P table update is
higher. I will change it
in the RFC patch.
thanks,
Bean
