| Message ID | 1567523655-23989-3-git-send-email-maxg@mellanox.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] block: centrelize PI remapping logic to the block layer | expand |
> The nvme_cleanup_cmd function should be called to avoid resource leakage > (it's the opposite to nvme_setup_cmd). Fix the error flow during command > submission and also fix the missing call in command completion. Is it always called with nvme_complete_rq? Why not just put it there?
On Tue, Sep 03, 2019 at 12:15:48PM -0700, Sagi Grimberg wrote: > >> The nvme_cleanup_cmd function should be called to avoid resource leakage >> (it's the opposite to nvme_setup_cmd). Fix the error flow during command >> submission and also fix the missing call in command completion. > > Is it always called with nvme_complete_rq? Why not just put it there? Yes, unless I am missing something we could call nvme_cleanup_cmd at the beginning of nvme_complete_rq. Max, can you send one series for all the nvme_cleanup_cmd fixes and cleanups and split that from the PI work? That might be a little less confusing.
On 9/4/2019 8:54 AM, Christoph Hellwig wrote: > On Tue, Sep 03, 2019 at 12:15:48PM -0700, Sagi Grimberg wrote: >>> The nvme_cleanup_cmd function should be called to avoid resource leakage >>> (it's the opposite to nvme_setup_cmd). Fix the error flow during command >>> submission and also fix the missing call in command completion. >> Is it always called with nvme_complete_rq? Why not just put it there? > Yes, unless I am missing something we could call nvme_cleanup_cmd > at the beginning of nvme_complete_rq. This will cause change is error flow in nvme_fc but I can check this. > > Max, can you send one series for all the nvme_cleanup_cmd fixes and > cleanups and split that from the PI work? That might be a little > less confusing. Yes I will. There is a connection between the patches but for now only the nvme-pci supports T10 in the nvme subsystem, we can separate them. There will be still a small gap in the error flow of the pci driver that will call nvme_cleanup_cmd and do the t10 remap that he shouldn't (but that's the behavior today)
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 606b13d..91b500a 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -2093,6 +2093,7 @@ static blk_status_t nvme_tcp_setup_cmd_pdu(struct nvme_ns *ns, ret = nvme_tcp_map_data(queue, rq); if (unlikely(ret)) { + nvme_cleanup_cmd(rq); dev_err(queue->ctrl->ctrl.device, "Failed to map data (%d)\n", ret); return ret; @@ -2101,6 +2102,12 @@ static blk_status_t nvme_tcp_setup_cmd_pdu(struct nvme_ns *ns, return 0; } +static void nvme_tcp_complete_rq(struct request *rq) +{ + nvme_cleanup_cmd(rq); + nvme_complete_rq(rq); +} + static blk_status_t nvme_tcp_queue_rq(struct blk_mq_hw_ctx *hctx, const struct blk_mq_queue_data *bd) { @@ -2161,7 +2168,7 @@ static int nvme_tcp_map_queues(struct blk_mq_tag_set *set) static struct blk_mq_ops nvme_tcp_mq_ops = { .queue_rq = nvme_tcp_queue_rq, - .complete = nvme_complete_rq, + .complete = nvme_tcp_complete_rq, .init_request = nvme_tcp_init_request, .exit_request = nvme_tcp_exit_request, .init_hctx = nvme_tcp_init_hctx, @@ -2171,7 +2178,7 @@ static int nvme_tcp_map_queues(struct blk_mq_tag_set *set) static struct blk_mq_ops nvme_tcp_admin_mq_ops = { .queue_rq = nvme_tcp_queue_rq, - .complete = nvme_complete_rq, + .complete = nvme_tcp_complete_rq, .init_request = nvme_tcp_init_request, .exit_request = nvme_tcp_exit_request, .init_hctx = nvme_tcp_init_admin_hctx,
The nvme_cleanup_cmd function should be called to avoid resource leakage (it's the opposite to nvme_setup_cmd). Fix the error flow during command submission and also fix the missing call in command completion. Signed-off-by: Max Gurtovoy <maxg@mellanox.com> --- drivers/nvme/host/tcp.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)