[PATCH 2/5] nvme-core: add a function to submit a cancel command

[Sagi Grimberg]

On 10/05/2024 19:30, Maurizio Lombardi wrote:
> Add a function to send a cancel command to abort the specified request.
> To execute the task, the "single command" cancel action is used.
>
> When the cancel command completes, the host driver will print the
> number of deferred and immediate aborts performed by the target.
>
> Signed-off-by: Maurizio Lombardi <mlombard at redhat.com>
> ---
>   drivers/nvme/host/core.c | 54 ++++++++++++++++++++++++++++++++++++++++
>   drivers/nvme/host/nvme.h |  2 ++
>   2 files changed, 56 insertions(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index b48967c98114..3d8c31d5f8ae 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2965,6 +2965,60 @@ int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp, u8 csi,
>   	return nvme_submit_sync_cmd(ctrl->admin_q, &c, log, size);
>   }
>   
> +static enum rq_end_io_ret nvme_cancel_endio(struct request *req, blk_status_t error)
> +{
> +	struct nvme_ctrl *ctrl = req->end_io_data;
> +	u32 result;
> +	u16 imm_abrts, def_abrts;
> +	u16 status = nvme_req(req)->status;
> +
> +	result = le32_to_cpu(nvme_req(req)->result.u32);
> +
> +	def_abrts = upper_16_bits(result);
> +	imm_abrts = lower_16_bits(result);
> +
> +	dev_warn(ctrl->device,
> +		 "Cancel status: 0x%x imm abrts = %u def abrts = %u",
> +		 status, imm_abrts, def_abrts);
> +
> +	blk_mq_free_request(req);
> +	return RQ_END_IO_NONE;
> +}
> +
> +int nvme_submit_cancel_req(struct nvme_ctrl *ctrl, struct request *rq,
> +			   unsigned int sqid)
> +{
> +	struct nvme_command c = { };
> +	struct request *cancel_req;
> +
> +	if (sqid == 0)
> +		return -EINVAL;
> +
> +	c.cancel.opcode = nvme_cmd_cancel;
> +	c.cancel.cid = nvme_cid(rq);
> +	c.cancel.sqid = cpu_to_le32(sqid);
> +	c.cancel.nsid = NVME_NSID_ALL;
> +	c.cancel.action = NVME_CANCEL_ACTION_SINGLE_CMD;
> +
> +	cancel_req = blk_mq_alloc_request_hctx(rq->q, nvme_req_op(&c),

I know that people discouraged expanding the use of 
blk_mq_alloc_request_hctx in the past, but I think it is warranted here. 
Note that it does not play well with cpu hotplug code (we have the same 
issue with nvmf connect).
> +					       BLK_MQ_REQ_NOWAIT |
> +					       BLK_MQ_REQ_RESERVED,
> +					       sqid - 1);

I think we need to account for a new reserved commands that is now
can be taken outside of connect. Should this be reserved at all?
meaning even if we account for it, there are potentially more and more
requests that we'd want to cancel...

We just seen why not reserving requests for connect is in:
de105068fead ("nvme: fix reconnection fail due to reserved tag allocation")