Re: [RFC PATCH 4/5] scsi: ufs: L2P map management for HPB read

Bart Van Assche
2020-06-13T15:24:10Z

On 2020-06-11 20:37, Daejun Park wrote:
>>> +static int ufshpb_execute_map_req(struct ufshpb_lu *hpb,
>>> + struct ufshpb_req *map_req)
>>> +{
>>> + struct request_queue *q;
>>> + struct request *req;
>>> + struct scsi_request *rq;
>>> + int ret = 0;
>>> +
>>> + q = hpb->sdev_ufs_lu->request_queue;
>>> + ret = ufshpb_map_req_add_bio_page(hpb, q, map_req->bio,
>>> + map_req->mctx);
>>> + if (ret) {
>>> + dev_notice(&hpb->hpb_lu_dev,
>>> + "map_req_add_bio_page fail %d - %d\n",
>>> + map_req->rgn_idx, map_req->srgn_idx);
>>> + return ret;
>>> + }
>>> +
>>> + req = map_req->req;
>>> +
>>> + blk_rq_append_bio(req, &map_req->bio);
>>> + req->rq_flags |= RQF_QUIET;
>>> + req->timeout = MAP_REQ_TIMEOUT;
>>> + req->end_io_data = (void *)map_req;
>>> +
>>> + rq = scsi_req(req);
>>> + ufshpb_set_read_buf_cmd(rq->cmd, map_req->rgn_idx,
>>> + map_req->srgn_idx, hpb->srgn_mem_size);
>>> + rq->cmd_len = HPB_READ_BUFFER_CMD_LENGTH;
>>> +
>>> + blk_execute_rq_nowait(q, NULL, req, 1, ufshpb_map_req_compl_fn);
>>> +
>>> + atomic_inc(&hpb->stats.map_req_cnt);
>>> + return 0;
>>> +}
>>
>> Why a custom timeout instead of the SCSI LUN timeout?
>
> There was no suitable timeout value to use. I've included sd.h, so I'll
> use sd_timeout.

Wouldn't that be a layering violation? The UFS driver is a SCSI LLD
driver and the sd driver is a SCSI ULD. A SCSI LLD must not make any
assumptions about which ULD driver has been attached.

How about leaving req->timeout zero such that blk_add_timer() sets it?
blk_add_timer() is called by blk_mq_start_request(). From blk_add_timer():

if (!req->timeout)
req->timeout = q->rq_timeout;

Thanks,

Bart.