* [PATCH] nvme-tcp: fix usage of page_frag_cache
@ 2025-09-29 11:19 Dmitry Bogdanov
2025-10-01 6:31 ` Chris Leech
2025-10-01 6:34 ` [PATCH] nvme-tcp: switch to per-cpu page_frag_cache Chris Leech
0 siblings, 2 replies; 8+ messages in thread
From: Dmitry Bogdanov @ 2025-09-29 11:19 UTC (permalink / raw)
To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Stuart Hayes, linux-nvme, linux-kernel
Cc: linux, Dmitry Bogdanov, stable
nvme uses page_frag_cache to preallocate PDU for each preallocated request
of block device. Block devices are created in parallel threads,
consequently page_frag_cache is used in not thread-safe manner.
That leads to incorrect refcounting of backstore pages and premature free.
That can be catched by !sendpage_ok inside network stack:
WARNING: CPU: 7 PID: 467 at ../net/core/skbuff.c:6931 skb_splice_from_iter+0xfa/0x310.
tcp_sendmsg_locked+0x782/0xce0
tcp_sendmsg+0x27/0x40
sock_sendmsg+0x8b/0xa0
nvme_tcp_try_send_cmd_pdu+0x149/0x2a0
Then random panic may occur.
Fix that by serializing the usage of page_frag_cache.
Cc: stable@vger.kernel.org # 6.12
Fixes: 4e893ca81170 ("nvme_core: scan namespaces asynchronously")
Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
---
drivers/nvme/host/tcp.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 1413788ca7d52..823e07759e0d3 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -145,6 +145,7 @@ struct nvme_tcp_queue {
struct mutex queue_lock;
struct mutex send_mutex;
+ struct mutex pf_cache_lock;
struct llist_head req_list;
struct list_head send_list;
@@ -556,9 +557,11 @@ static int nvme_tcp_init_request(struct blk_mq_tag_set *set,
struct nvme_tcp_queue *queue = &ctrl->queues[queue_idx];
u8 hdgst = nvme_tcp_hdgst_len(queue);
+ mutex_lock(&queue->pf_cache_lock);
req->pdu = page_frag_alloc(&queue->pf_cache,
sizeof(struct nvme_tcp_cmd_pdu) + hdgst,
GFP_KERNEL | __GFP_ZERO);
+ mutex_unlock(&queue->pf_cache_lock);
if (!req->pdu)
return -ENOMEM;
@@ -1420,9 +1423,11 @@ static int nvme_tcp_alloc_async_req(struct nvme_tcp_ctrl *ctrl)
struct nvme_tcp_request *async = &ctrl->async_req;
u8 hdgst = nvme_tcp_hdgst_len(queue);
+ mutex_lock(&queue->pf_cache_lock);
async->pdu = page_frag_alloc(&queue->pf_cache,
sizeof(struct nvme_tcp_cmd_pdu) + hdgst,
GFP_KERNEL | __GFP_ZERO);
+ mutex_unlock(&queue->pf_cache_lock);
if (!async->pdu)
return -ENOMEM;
@@ -1450,6 +1455,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->pf_cache_lock);
}
static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
@@ -1772,6 +1778,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
INIT_LIST_HEAD(&queue->send_list);
mutex_init(&queue->send_mutex);
INIT_WORK(&queue->io_work, nvme_tcp_io_work);
+ mutex_init(&queue->pf_cache_lock);
if (qid > 0)
queue->cmnd_capsule_len = nctrl->ioccsz * 16;
@@ -1903,6 +1910,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
err_destroy_mutex:
mutex_destroy(&queue->send_mutex);
mutex_destroy(&queue->queue_lock);
+ mutex_destroy(&queue->pf_cache_lock);
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] nvme-tcp: fix usage of page_frag_cache
2025-09-29 11:19 [PATCH] nvme-tcp: fix usage of page_frag_cache Dmitry Bogdanov
@ 2025-10-01 6:31 ` Chris Leech
2025-10-01 16:41 ` Dmitry Bogdanov
2025-10-01 6:34 ` [PATCH] nvme-tcp: switch to per-cpu page_frag_cache Chris Leech
1 sibling, 1 reply; 8+ messages in thread
From: Chris Leech @ 2025-10-01 6:31 UTC (permalink / raw)
To: Dmitry Bogdanov
Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Stuart Hayes, linux-nvme, linux-kernel, linux, stable
On Mon, Sep 29, 2025 at 02:19:51PM +0300, Dmitry Bogdanov wrote:
> nvme uses page_frag_cache to preallocate PDU for each preallocated request
> of block device. Block devices are created in parallel threads,
> consequently page_frag_cache is used in not thread-safe manner.
> That leads to incorrect refcounting of backstore pages and premature free.
>
> That can be catched by !sendpage_ok inside network stack:
>
> WARNING: CPU: 7 PID: 467 at ../net/core/skbuff.c:6931 skb_splice_from_iter+0xfa/0x310.
> tcp_sendmsg_locked+0x782/0xce0
> tcp_sendmsg+0x27/0x40
> sock_sendmsg+0x8b/0xa0
> nvme_tcp_try_send_cmd_pdu+0x149/0x2a0
> Then random panic may occur.
>
> Fix that by serializing the usage of page_frag_cache.
Thank you for reporting this. I think we can fix it without blocking the
async namespace scanning with a mutex, by switching from a per-queue
page_frag_cache to per-cpu. There shouldn't be a need to keep the
page_frag allocations isolated by queue anyway.
It would be great if you could test the patch which I'll send after
this.
- Chris
> Cc: stable@vger.kernel.org # 6.12
> Fixes: 4e893ca81170 ("nvme_core: scan namespaces asynchronously")
> Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
> ---
> drivers/nvme/host/tcp.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 1413788ca7d52..823e07759e0d3 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -145,6 +145,7 @@ struct nvme_tcp_queue {
>
> struct mutex queue_lock;
> struct mutex send_mutex;
> + struct mutex pf_cache_lock;
> struct llist_head req_list;
> struct list_head send_list;
>
> @@ -556,9 +557,11 @@ static int nvme_tcp_init_request(struct blk_mq_tag_set *set,
> struct nvme_tcp_queue *queue = &ctrl->queues[queue_idx];
> u8 hdgst = nvme_tcp_hdgst_len(queue);
>
> + mutex_lock(&queue->pf_cache_lock);
> req->pdu = page_frag_alloc(&queue->pf_cache,
> sizeof(struct nvme_tcp_cmd_pdu) + hdgst,
> GFP_KERNEL | __GFP_ZERO);
> + mutex_unlock(&queue->pf_cache_lock);
> if (!req->pdu)
> return -ENOMEM;
>
> @@ -1420,9 +1423,11 @@ static int nvme_tcp_alloc_async_req(struct nvme_tcp_ctrl *ctrl)
> struct nvme_tcp_request *async = &ctrl->async_req;
> u8 hdgst = nvme_tcp_hdgst_len(queue);
>
> + mutex_lock(&queue->pf_cache_lock);
> async->pdu = page_frag_alloc(&queue->pf_cache,
> sizeof(struct nvme_tcp_cmd_pdu) + hdgst,
> GFP_KERNEL | __GFP_ZERO);
> + mutex_unlock(&queue->pf_cache_lock);
> if (!async->pdu)
> return -ENOMEM;
>
> @@ -1450,6 +1455,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->pf_cache_lock);
> }
>
> static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
> @@ -1772,6 +1778,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
> INIT_LIST_HEAD(&queue->send_list);
> mutex_init(&queue->send_mutex);
> INIT_WORK(&queue->io_work, nvme_tcp_io_work);
> + mutex_init(&queue->pf_cache_lock);
>
> if (qid > 0)
> queue->cmnd_capsule_len = nctrl->ioccsz * 16;
> @@ -1903,6 +1910,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
> err_destroy_mutex:
> mutex_destroy(&queue->send_mutex);
> mutex_destroy(&queue->queue_lock);
> + mutex_destroy(&queue->pf_cache_lock);
> return ret;
> }
>
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] nvme-tcp: fix usage of page_frag_cache
2025-10-01 6:31 ` Chris Leech
@ 2025-10-01 16:41 ` Dmitry Bogdanov
0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Bogdanov @ 2025-10-01 16:41 UTC (permalink / raw)
To: Chris Leech
Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Stuart Hayes, linux-nvme, linux-kernel, linux, stable
On Tue, Sep 30, 2025 at 11:31:26PM -0700, Chris Leech wrote:
>
> On Mon, Sep 29, 2025 at 02:19:51PM +0300, Dmitry Bogdanov wrote:
> > nvme uses page_frag_cache to preallocate PDU for each preallocated request
> > of block device. Block devices are created in parallel threads,
> > consequently page_frag_cache is used in not thread-safe manner.
> > That leads to incorrect refcounting of backstore pages and premature free.
> >
> > That can be catched by !sendpage_ok inside network stack:
> >
> > WARNING: CPU: 7 PID: 467 at ../net/core/skbuff.c:6931 skb_splice_from_iter+0xfa/0x310.
> > tcp_sendmsg_locked+0x782/0xce0
> > tcp_sendmsg+0x27/0x40
> > sock_sendmsg+0x8b/0xa0
> > nvme_tcp_try_send_cmd_pdu+0x149/0x2a0
> > Then random panic may occur.
> >
> > Fix that by serializing the usage of page_frag_cache.
>
> Thank you for reporting this. I think we can fix it without blocking the
> async namespace scanning with a mutex, by switching from a per-queue
> page_frag_cache to per-cpu. There shouldn't be a need to keep the
> page_frag allocations isolated by queue anyway.
>
> It would be great if you could test the patch which I'll send after
> this.
>
As I commented on your patch, a naive per-cpu cache solution is
error-prone. The complete solution will be unnecessaryly difficult.
Block device creation is not a data plane, it is a control plane, so
there is no sense to use there lockless algorithms.
My patch is a simple and error-proof already.
So, I insist on this solution.
BR,
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme-tcp: switch to per-cpu page_frag_cache
2025-09-29 11:19 [PATCH] nvme-tcp: fix usage of page_frag_cache Dmitry Bogdanov
2025-10-01 6:31 ` Chris Leech
@ 2025-10-01 6:34 ` Chris Leech
2025-10-01 16:24 ` Dmitry Bogdanov
1 sibling, 1 reply; 8+ messages in thread
From: Chris Leech @ 2025-10-01 6:34 UTC (permalink / raw)
To: Dmitry Bogdanov
Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Stuart Hayes, linux-nvme, linux-kernel, linux, stable
nvme-tcp uses page_frag_cache to preallocate PDU for each preallocated
request of block device. Block devices are created in parallel threads,
consequently page_frag_cache is used in not thread-safe manner.
That leads to incorrect refcounting of backstore pages and premature free.
That can be catched by !sendpage_ok inside network stack:
WARNING: CPU: 7 PID: 467 at ../net/core/skbuff.c:6931 skb_splice_from_iter+0xfa/0x310.
tcp_sendmsg_locked+0x782/0xce0
tcp_sendmsg+0x27/0x40
sock_sendmsg+0x8b/0xa0
nvme_tcp_try_send_cmd_pdu+0x149/0x2a0
Then random panic may occur.
Fix that by switching from having a per-queue page_frag_cache to a
per-cpu page_frag_cache.
Cc: stable@vger.kernel.org # 6.12
Fixes: 4e893ca81170 ("nvme_core: scan namespaces asynchronously")
Reported-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
Signed-off-by: Chris Leech <cleech@redhat.com>
---
drivers/nvme/host/tcp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 1413788ca7d52..a4c4ace5be0f4 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -174,7 +174,6 @@ struct nvme_tcp_queue {
__le32 recv_ddgst;
struct completion tls_complete;
int tls_err;
- struct page_frag_cache pf_cache;
void (*state_change)(struct sock *);
void (*data_ready)(struct sock *);
@@ -201,6 +200,7 @@ struct nvme_tcp_ctrl {
static LIST_HEAD(nvme_tcp_ctrl_list);
static DEFINE_MUTEX(nvme_tcp_ctrl_mutex);
+static DEFINE_PER_CPU(struct page_frag_cache, pf_cache);
static struct workqueue_struct *nvme_tcp_wq;
static const struct blk_mq_ops nvme_tcp_mq_ops;
static const struct blk_mq_ops nvme_tcp_admin_mq_ops;
@@ -556,7 +556,7 @@ static int nvme_tcp_init_request(struct blk_mq_tag_set *set,
struct nvme_tcp_queue *queue = &ctrl->queues[queue_idx];
u8 hdgst = nvme_tcp_hdgst_len(queue);
- req->pdu = page_frag_alloc(&queue->pf_cache,
+ req->pdu = page_frag_alloc(this_cpu_ptr(&pf_cache),
sizeof(struct nvme_tcp_cmd_pdu) + hdgst,
GFP_KERNEL | __GFP_ZERO);
if (!req->pdu)
@@ -1420,7 +1420,7 @@ static int nvme_tcp_alloc_async_req(struct nvme_tcp_ctrl *ctrl)
struct nvme_tcp_request *async = &ctrl->async_req;
u8 hdgst = nvme_tcp_hdgst_len(queue);
- async->pdu = page_frag_alloc(&queue->pf_cache,
+ async->pdu = page_frag_alloc(this_cpu_ptr(&pf_cache),
sizeof(struct nvme_tcp_cmd_pdu) + hdgst,
GFP_KERNEL | __GFP_ZERO);
if (!async->pdu)
@@ -1439,7 +1439,7 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
if (!test_and_clear_bit(NVME_TCP_Q_ALLOCATED, &queue->flags))
return;
- page_frag_cache_drain(&queue->pf_cache);
+ page_frag_cache_drain(this_cpu_ptr(&pf_cache));
noreclaim_flag = memalloc_noreclaim_save();
/* ->sock will be released by fput() */
--
2.50.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] nvme-tcp: switch to per-cpu page_frag_cache
2025-10-01 6:34 ` [PATCH] nvme-tcp: switch to per-cpu page_frag_cache Chris Leech
@ 2025-10-01 16:24 ` Dmitry Bogdanov
0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Bogdanov @ 2025-10-01 16:24 UTC (permalink / raw)
To: Chris Leech
Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Stuart Hayes, linux-nvme, linux-kernel, linux, stable
On Tue, Sep 30, 2025 at 11:34:14PM -0700, Chris Leech wrote:
>
> nvme-tcp uses page_frag_cache to preallocate PDU for each preallocated
> request of block device. Block devices are created in parallel threads,
> consequently page_frag_cache is used in not thread-safe manner.
> That leads to incorrect refcounting of backstore pages and premature free.
>
> That can be catched by !sendpage_ok inside network stack:
>
> WARNING: CPU: 7 PID: 467 at ../net/core/skbuff.c:6931 skb_splice_from_iter+0xfa/0x310.
> tcp_sendmsg_locked+0x782/0xce0
> tcp_sendmsg+0x27/0x40
> sock_sendmsg+0x8b/0xa0
> nvme_tcp_try_send_cmd_pdu+0x149/0x2a0
> Then random panic may occur.
>
> Fix that by switching from having a per-queue page_frag_cache to a
> per-cpu page_frag_cache.
>
> Cc: stable@vger.kernel.org # 6.12
> Fixes: 4e893ca81170 ("nvme_core: scan namespaces asynchronously")
> Reported-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
> Signed-off-by: Chris Leech <cleech@redhat.com>
> ---
> drivers/nvme/host/tcp.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 1413788ca7d52..a4c4ace5be0f4 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -174,7 +174,6 @@ struct nvme_tcp_queue {
> __le32 recv_ddgst;
> struct completion tls_complete;
> int tls_err;
> - struct page_frag_cache pf_cache;
>
> void (*state_change)(struct sock *);
> void (*data_ready)(struct sock *);
> @@ -201,6 +200,7 @@ struct nvme_tcp_ctrl {
>
> static LIST_HEAD(nvme_tcp_ctrl_list);
> static DEFINE_MUTEX(nvme_tcp_ctrl_mutex);
> +static DEFINE_PER_CPU(struct page_frag_cache, pf_cache);
> static struct workqueue_struct *nvme_tcp_wq;
> static const struct blk_mq_ops nvme_tcp_mq_ops;
> static const struct blk_mq_ops nvme_tcp_admin_mq_ops;
> @@ -556,7 +556,7 @@ static int nvme_tcp_init_request(struct blk_mq_tag_set *set,
> struct nvme_tcp_queue *queue = &ctrl->queues[queue_idx];
> u8 hdgst = nvme_tcp_hdgst_len(queue);
>
> - req->pdu = page_frag_alloc(&queue->pf_cache,
> + req->pdu = page_frag_alloc(this_cpu_ptr(&pf_cache),
> sizeof(struct nvme_tcp_cmd_pdu) + hdgst,
> GFP_KERNEL | __GFP_ZERO);
I am not good at scheduler subsystem, but as far as I understand,
workqueues may execute its work items in parallel up to max_active work
items on the same CPU. It means that this solution does not fix the
issue of parallel usage of the same variable.
Can anyone comment on this?
> if (!req->pdu)
> @@ -1420,7 +1420,7 @@ static int nvme_tcp_alloc_async_req(struct nvme_tcp_ctrl *ctrl)
> struct nvme_tcp_request *async = &ctrl->async_req;
> u8 hdgst = nvme_tcp_hdgst_len(queue);
>
> - async->pdu = page_frag_alloc(&queue->pf_cache,
> + async->pdu = page_frag_alloc(this_cpu_ptr(&pf_cache),
> sizeof(struct nvme_tcp_cmd_pdu) + hdgst,
> GFP_KERNEL | __GFP_ZERO);
This line is executed in a different(parallel) context comparing to
nvme_tcp_init_request.
> if (!async->pdu)
> @@ -1439,7 +1439,7 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
> if (!test_and_clear_bit(NVME_TCP_Q_ALLOCATED, &queue->flags))
> return;
>
> - page_frag_cache_drain(&queue->pf_cache);
> + page_frag_cache_drain(this_cpu_ptr(&pf_cache));
This line is also definitely processed in other(parallel) context comparing
to nvme_tcp_init_request. And frees the still used pages by other queues.
> noreclaim_flag = memalloc_noreclaim_save();
> /* ->sock will be released by fput() */
> --
> 2.50.1
>
In total, my patch with a mutex looks more appropriate and more error-proof.
BR,
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] nvme-tcp: fix usage of page_frag_cache
@ 2026-07-16 14:42 Daniel Wagner
2026-08-06 12:46 ` Daniel Wagner
2026-08-10 23:40 ` Keith Busch
0 siblings, 2 replies; 8+ messages in thread
From: Daniel Wagner @ 2026-07-16 14:42 UTC (permalink / raw)
To: Keith Busch, Christoph Hellwig, Sagi Grimberg
Cc: Chris Leech, linux-nvme, linux-kernel, Dmitry Bogdanov,
Daniel Wagner
From: Dmitry Bogdanov <d.bogdanov@yadro.com>
nvme uses page_frag_cache to preallocate PDU for each preallocated request
of block device. Block devices are created in parallel threads,
consequently page_frag_cache is used in not thread-safe manner.
That leads to incorrect refcounting of backstore pages and premature free.
That can be catched by !sendpage_ok inside network stack:
WARNING: CPU: 7 PID: 467 at ../net/core/skbuff.c:6931 skb_splice_from_iter+0xfa/0x310.
tcp_sendmsg_locked+0x782/0xce0
tcp_sendmsg+0x27/0x40
sock_sendmsg+0x8b/0xa0
nvme_tcp_try_send_cmd_pdu+0x149/0x2a0
Then random panic may occur.
Fix that by serializing the usage of page_frag_cache.
Fixes: 4e893ca81170 ("nvme_core: scan namespaces asynchronously")
Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
If the target exposes many namespaces (>1000) and the host has many CPUs (>80),
it is trivial to trigger the allocation race condition in nvme_tcp_init_request
which results in the logs below:
WARNING: CPU: XX PID: XXXX at net/core/skbuff.c:XXXX skb_splice_from_iter+0xfa/0x310
nvme nvme22: failed to send request -5
nvme nvme23: failed to send request -5
nvme nvme24: failed to send request -5
[... repeating for all controllers, thousands of times ...]
RDX: 00000000000005e8 RSI: 0000000000000010 RDI: 0000000000000000
RBP: 000000000000004c R08: ff57ae906085bd78 R09: 000000000000004c
R10: 000000000000004c R11: 00000000000003ef R12: 0000000000000000
R13: ff4f98b626e30c00 R14: ff57ae906085bbf0 R15: ff4f98b626e30c00
FS: 0000000000000000(0000) GS:ff4f98e2abc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f0f167ff000 CR3: 0000000382a18003 CR4: 0000000000f71ee0
Call Trace:
<TASK>
? __warn+0x86/0x150
? skb_splice_from_iter+0xfa/0x310
? report_bug+0xfb/0x1e0
? handle_bug+0x44/0x80
? exc_invalid_op+0x13/0x60
? asm_exc_invalid_op+0x16/0x20
? skb_splice_from_iter+0xfa/0x310
? __alloc_skb+0xd5/0x190
tcp_sendmsg_locked+0x782/0xcd0
tcp_sendmsg+0x27/0x40
sock_sendmsg+0x98/0xc0
nvme_tcp_try_send_cmd_pdu+0x149/0x2a0 [nvme_tcp]
nvme_tcp_try_send+0xbb/0x2c0 [nvme_tcp]
nvme_tcp_io_work+0x37/0xb0 [nvme_tcp]
process_one_work+0x223/0x460
? __pfx_worker_thread+0x10/0x10
worker_thread+0x2a/0x3b0
? __pfx_worker_thread+0x10/0x10
kthread+0xdf/0x120
? __pfx_kthread+0x10/0x10
ret_from_fork+0x29/0x50
</TASK>
The above excerpt is from our customers log. I was able to reproduce this on the
latest nvme-7.3 branch. Dmitry's patch [1] looks correct to me. All the
allocation happens in the slow path and the context can sleep, thus a mutex
seems to be a good choice.
[1] https://lore.kernel.org/linux-nvme/20250929111951.6961-1-d.bogdanov@yadro.com/
---
drivers/nvme/host/tcp.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index ba5c7b3e2a7c..dd40798bc248 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -108,6 +108,7 @@ struct nvme_tcp_queue {
struct mutex queue_lock;
struct mutex send_mutex;
+ struct mutex pf_cache_lock;
struct llist_head req_list;
struct list_head send_list;
@@ -550,9 +551,11 @@ static int nvme_tcp_init_request(struct blk_mq_tag_set *set,
struct nvme_tcp_queue *queue = &ctrl->queues[queue_idx];
u8 hdgst = nvme_tcp_hdgst_len(queue);
+ mutex_lock(&queue->pf_cache_lock);
req->pdu = page_frag_alloc(&queue->pf_cache,
sizeof(struct nvme_tcp_cmd_pdu) + hdgst,
GFP_KERNEL | __GFP_ZERO);
+ mutex_unlock(&queue->pf_cache_lock);
if (!req->pdu)
return -ENOMEM;
@@ -1417,9 +1420,11 @@ static int nvme_tcp_alloc_async_req(struct nvme_tcp_ctrl *ctrl)
struct nvme_tcp_request *async = &ctrl->async_req;
u8 hdgst = nvme_tcp_hdgst_len(queue);
+ mutex_lock(&queue->pf_cache_lock);
async->pdu = page_frag_alloc(&queue->pf_cache,
sizeof(struct nvme_tcp_cmd_pdu) + hdgst,
GFP_KERNEL | __GFP_ZERO);
+ mutex_unlock(&queue->pf_cache_lock);
if (!async->pdu)
return -ENOMEM;
@@ -1461,6 +1466,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->pf_cache_lock);
#ifdef CONFIG_DEBUG_LOCK_ALLOC
lockdep_unregister_key(&queue->nvme_tcp_sk_key);
@@ -1788,6 +1794,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
INIT_LIST_HEAD(&queue->send_list);
mutex_init(&queue->send_mutex);
INIT_WORK(&queue->io_work, nvme_tcp_io_work);
+ mutex_init(&queue->pf_cache_lock);
if (qid > 0)
queue->cmnd_capsule_len = nctrl->ioccsz * 16;
@@ -1928,6 +1935,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
err_destroy_mutex:
mutex_destroy(&queue->send_mutex);
mutex_destroy(&queue->queue_lock);
+ mutex_destroy(&queue->pf_cache_lock);
return ret;
}
---
base-commit: 29261f8bb41662f2a660c479e5cf592942b53f78
change-id: 20260716-nvme-tcp-page_fraq_cache-d04f6226f927
Best regards,
--
Daniel Wagner <wagi@kernel.org>
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] nvme-tcp: fix usage of page_frag_cache
2026-07-16 14:42 [PATCH] nvme-tcp: fix usage of page_frag_cache Daniel Wagner
@ 2026-08-06 12:46 ` Daniel Wagner
2026-08-10 23:40 ` Keith Busch
1 sibling, 0 replies; 8+ messages in thread
From: Daniel Wagner @ 2026-08-06 12:46 UTC (permalink / raw)
To: Keith Busch, Christoph Hellwig, Sagi Grimberg
Cc: Chris Leech, linux-nvme, linux-kernel, Dmitry Bogdanov
On Thu, Jul 16, 2026 at 04:42:19PM +0200, Daniel Wagner wrote:
> From: Dmitry Bogdanov <d.bogdanov@yadro.com>
>
> nvme uses page_frag_cache to preallocate PDU for each preallocated request
> of block device. Block devices are created in parallel threads,
> consequently page_frag_cache is used in not thread-safe manner.
> That leads to incorrect refcounting of backstore pages and premature free.
>
> That can be catched by !sendpage_ok inside network stack:
>
> WARNING: CPU: 7 PID: 467 at ../net/core/skbuff.c:6931 skb_splice_from_iter+0xfa/0x310.
> tcp_sendmsg_locked+0x782/0xce0
> tcp_sendmsg+0x27/0x40
> sock_sendmsg+0x8b/0xa0
> nvme_tcp_try_send_cmd_pdu+0x149/0x2a0
> Then random panic may occur.
>
> Fix that by serializing the usage of page_frag_cache.
>
> Fixes: 4e893ca81170 ("nvme_core: scan namespaces asynchronously")
> Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
> Signed-off-by: Daniel Wagner <wagi@kernel.org>
> ---
> If the target exposes many namespaces (>1000) and the host has many CPUs (>80),
> it is trivial to trigger the allocation race condition in nvme_tcp_init_request
> which results in the logs below:
Any chance to move forward with this on?
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] nvme-tcp: fix usage of page_frag_cache
2026-07-16 14:42 [PATCH] nvme-tcp: fix usage of page_frag_cache Daniel Wagner
2026-08-06 12:46 ` Daniel Wagner
@ 2026-08-10 23:40 ` Keith Busch
1 sibling, 0 replies; 8+ messages in thread
From: Keith Busch @ 2026-08-10 23:40 UTC (permalink / raw)
To: Daniel Wagner
Cc: Christoph Hellwig, Sagi Grimberg, Chris Leech, linux-nvme,
linux-kernel, Dmitry Bogdanov, Daniel Wagner
On Thu, Jul 16, 2026 at 04:42:19PM +0200, Daniel Wagner wrote:
> From: Dmitry Bogdanov <d.bogdanov@yadro.com>
>
> nvme uses page_frag_cache to preallocate PDU for each preallocated request
> of block device. Block devices are created in parallel threads,
> consequently page_frag_cache is used in not thread-safe manner.
> That leads to incorrect refcounting of backstore pages and premature free.
I was struggling to see how the nvme_tcp callback was happening
concurrently for the same queue. It's because you're using an io
scheduler, right? I don't think this applies if you don't have one
enabled.
Anyway, with that understanding, looks like a safe patch. Applied to
nvme-7.3.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-10 23:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-29 11:19 [PATCH] nvme-tcp: fix usage of page_frag_cache Dmitry Bogdanov
2025-10-01 6:31 ` Chris Leech
2025-10-01 16:41 ` Dmitry Bogdanov
2025-10-01 6:34 ` [PATCH] nvme-tcp: switch to per-cpu page_frag_cache Chris Leech
2025-10-01 16:24 ` Dmitry Bogdanov
-- strict thread matches above, loose matches on Subject: below --
2026-07-16 14:42 [PATCH] nvme-tcp: fix usage of page_frag_cache Daniel Wagner
2026-08-06 12:46 ` Daniel Wagner
2026-08-10 23:40 ` Keith Busch
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.