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

[Bart Van Assche <bvanassche@acm.org>]

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.