On Wed Feb 12, 2025 at 11:28 AM CET, Maurizio Lombardi wrote:
> On Wed Feb 12, 2025 at 10:47 AM CET, zhang.guanghui@xxxxxxxx wrote:
>> Hi, Thanks.
>> I will test this patch, but I am worried whether it will affect the performance.
>> Should we also consider null pointer protection?
>
> Yes, it will likely affect the performance, just check if it works.
>
> Probably it could be optimized by just protecting
> nvme_tcp_fail_request(), which AFAICT is the only function in the
> nvme_tcp_try_send() code that calls nvme_complete_rq().
Something like that, maybe, not tested:
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 841238f38fdd..488edec35a65 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -146,6 +146,7 @@ struct nvme_tcp_queue {
struct mutex queue_lock;
struct mutex send_mutex;
+ struct mutex poll_mutex;
struct llist_head req_list;
struct list_head send_list;
@@ -1259,7 +1260,9 @@ static int nvme_tcp_try_send(struct nvme_tcp_queue *queue)
} else if (ret < 0) {
dev_err(queue->ctrl->ctrl.device,
"failed to send request %d\n", ret);
+ mutex_lock(&queue->poll_mutex);
nvme_tcp_fail_request(queue->request);
+ mutex_unlock(&queue->poll_mutex);
nvme_tcp_done_send_req(queue);
}
out:
@@ -1397,6 +1400,7 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
kfree(queue->pdu);
mutex_destroy(&queue->send_mutex);
mutex_destroy(&queue->queue_lock);
+ mutex_destroy(&queue->poll_mutex);
}
static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
@@ -1710,6 +1714,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
init_llist_head(&queue->req_list);
INIT_LIST_HEAD(&queue->send_list);
mutex_init(&queue->send_mutex);
+ mutex_init(&queue->poll_mutex);
INIT_WORK(&queue->io_work, nvme_tcp_io_work);
if (qid > 0)
@@ -2660,7 +2665,9 @@ static int nvme_tcp_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
set_bit(NVME_TCP_Q_POLLING, &queue->flags);
if (sk_can_busy_loop(sk) && skb_queue_empty_lockless(&sk->sk_receive_queue))
sk_busy_loop(sk, true);
+ mutex_lock(&queue->poll_mutex);
nvme_tcp_try_recv(queue);
+ mutex_unlock(&queue->poll_mutex);
clear_bit(NVME_TCP_Q_POLLING, &queue->flags);
return queue->nr_cqe;
}
