On Wed, Oct 21, 2020 at 06:02:30PM -0700, Chaitanya Kulkarni wrote:
> +static inline unsigned int nvme_req_op(struct nvme_command *cmd)
> +{
> + return nvme_is_write(cmd) ? REQ_OP_DRV_OUT : REQ_OP_DRV_IN;
> +}
Why is this added here while nvme_init_req_from_cmd is added in a prep
patch? I'm actually fine either way, but doing it differnetly for the
different helpers is a little inconsistent.
> +
> +struct request *nvme_alloc_request_qid_any(struct request_queue *q,
> + struct nvme_command *cmd, blk_mq_req_flags_t flags)
I'd call this just nvme_alloc_request to keep the short name for the
normal use case.
> + struct request *req;
> +
> + req = blk_mq_alloc_request(q, nvme_req_op(cmd), flags);
> + if (unlikely(IS_ERR(req)))
> + return req;
> +
> + nvme_init_req_from_cmd(req, cmd);
> + return req;
Could be simplified to:
req = blk_mq_alloc_request(q, nvme_req_op(cmd), flags);
if (!IS_ERR(req))
nvme_init_req_from_cmd(req, cmd);
return req;
Note that IS_ERR already contains an embedded unlikely().
> +static struct request *nvme_alloc_request_qid(struct request_queue *q,
> struct nvme_command *cmd, blk_mq_req_flags_t flags, int qid)
> {
> struct request *req;
>
> + req = blk_mq_alloc_request_hctx(q, nvme_req_op(cmd), flags,
> + qid ? qid - 1 : 0);
> if (IS_ERR(req))
> return req;
>
> nvme_init_req_from_cmd(req, cmd);
> return req;
Same here.
> }
> -EXPORT_SYMBOL_GPL(nvme_alloc_request);
I think nvme_alloc_request_qid needs to be exported as well.
FYI, this also doesn't apply to the current nvme-5.10 tree any more.
