* [PATCH v7 03/12] nvdimm: pmem: guard data loop for dataless bios
From: Li Chen @ 2026-06-30 9:23 UTC (permalink / raw)
To: Pankaj Gupta, Vishal Verma, Dave Jiang, Alison Schofield,
virtualization, nvdimm
Cc: linux-kernel, Li Chen
In-Reply-To: <20260630092338.2094628-1-me@linux.beauty>
pmem_submit_bio() handles flush-only bios before and after the data
loop. Keep dataless bios out of bio_for_each_segment() so the data path
only walks bios that actually carry bvec data.
Signed-off-by: Li Chen <me@linux.beauty>
---
Changes in v6:
- New patch.
drivers/nvdimm/pmem.c | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 05d3de33e2706..82ee1ddb3a445 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -217,23 +217,29 @@ static void pmem_submit_bio(struct bio *bio)
}
}
- do_acct = blk_queue_io_stat(bio->bi_bdev->bd_disk->queue);
- if (do_acct)
- start = bio_start_io_acct(bio);
- bio_for_each_segment(bvec, bio, iter) {
- if (op_is_write(bio_op(bio)))
- rc = pmem_do_write(pmem, bvec.bv_page, bvec.bv_offset,
- iter.bi_sector, bvec.bv_len);
- else
- rc = pmem_do_read(pmem, bvec.bv_page, bvec.bv_offset,
- iter.bi_sector, bvec.bv_len);
- if (rc) {
- bio->bi_status = rc;
- break;
+ if (bio_has_data(bio)) {
+ do_acct = blk_queue_io_stat(bio->bi_bdev->bd_disk->queue);
+ if (do_acct)
+ start = bio_start_io_acct(bio);
+ bio_for_each_segment(bvec, bio, iter) {
+ if (op_is_write(bio_op(bio)))
+ rc = pmem_do_write(pmem, bvec.bv_page,
+ bvec.bv_offset,
+ iter.bi_sector,
+ bvec.bv_len);
+ else
+ rc = pmem_do_read(pmem, bvec.bv_page,
+ bvec.bv_offset,
+ iter.bi_sector,
+ bvec.bv_len);
+ if (rc) {
+ bio->bi_status = rc;
+ break;
+ }
}
+ if (do_acct)
+ bio_end_io_acct(bio, start);
}
- if (do_acct)
- bio_end_io_acct(bio, start);
if ((bio->bi_opf & REQ_FUA) && !bio->bi_status)
ret = nvdimm_flush(nd_region, bio);
--
2.52.0
^ permalink raw reply related
* [PATCH v7 04/12] nvdimm: virtio_pmem: stop allocating child flush bio
From: Li Chen @ 2026-06-30 9:23 UTC (permalink / raw)
To: Pankaj Gupta, Vishal Verma, Dave Jiang, Alison Schofield,
virtualization, nvdimm
Cc: linux-kernel, Li Chen
In-Reply-To: <20260630092338.2094628-1-me@linux.beauty>
pmem_submit_bio() passes the parent bio to nvdimm_flush() for
REQ_FUA. For virtio-pmem this makes async_pmem_flush() allocate
and submit a child PREFLUSH bio chained to the parent.
That child allocation is in the block submit path. Making it
blocking with GFP_NOIO can consume the same global bio mempool that
submit_bio() uses, while making it GFP_ATOMIC can fail under
pressure. A forced failure of the child allocation produced:
virtio_pmem: forcing child bio allocation failure for test
Buffer I/O error on dev pmem0, logical block 0, lost sync page write
EXT4-fs (pmem0): I/O error while writing superblock
EXT4-fs (pmem0): mount failed
Avoid the child bio without turning REQ_FUA into a synchronous
submit-path wait. Let provider flush callbacks return
NVDIMM_FLUSH_ASYNC after taking ownership of parent bio completion.
pmem_submit_bio() returns in that case, and virtio-pmem queues an
ordered WQ_MEM_RECLAIM work item that runs the existing host flush
path and completes the parent bio.
This keeps the asynchronous completion model of the child-bio path
while removing the child bio allocation from the submit path.
Signed-off-by: Li Chen <me@linux.beauty>
---
Changes in v7:
- Replace synchronous FUA flushing with provider-owned asynchronous parent bio
completion.
- Add NVDIMM_FLUSH_ASYNC and ordered WQ_MEM_RECLAIM flush work.
Changes in v6:
- Replace the child bio allocation fix with synchronous FUA flushing.
drivers/nvdimm/nd_virtio.c | 54 +++++++++++++++++++++++++-----------
drivers/nvdimm/pmem.c | 5 +++-
drivers/nvdimm/region_devs.c | 2 ++
drivers/nvdimm/virtio_pmem.c | 17 +++++++++++-
drivers/nvdimm/virtio_pmem.h | 4 +++
include/linux/libnvdimm.h | 9 ++++++
6 files changed, 73 insertions(+), 18 deletions(-)
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index 4176046627beb..8e16b7780be1a 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -9,6 +9,12 @@
#include "virtio_pmem.h"
#include "nd.h"
+struct virtio_pmem_flush_work {
+ struct work_struct work;
+ struct nd_region *nd_region;
+ struct bio *bio;
+};
+
/* The interrupt handler */
void virtio_pmem_host_ack(struct virtqueue *vq)
{
@@ -107,30 +113,46 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
return err;
};
+static void virtio_pmem_flush_work(struct work_struct *work)
+{
+ struct virtio_pmem_flush_work *flush;
+ int err;
+
+ flush = container_of(work, struct virtio_pmem_flush_work, work);
+ err = virtio_pmem_flush(flush->nd_region);
+ if (err > 0)
+ err = -EIO;
+ if (err)
+ flush->bio->bi_status = errno_to_blk_status(err);
+ bio_endio(flush->bio);
+ kfree(flush);
+}
+
/* The asynchronous flush callback function */
int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
{
- /*
- * Create child bio for asynchronous flush and chain with
- * parent bio. Otherwise directly call nd_region flush.
- */
- if (bio && bio->bi_iter.bi_sector != -1) {
- struct bio *child = bio_alloc(bio->bi_bdev, 0,
- REQ_OP_WRITE | REQ_PREFLUSH,
- GFP_ATOMIC);
+ struct virtio_device *vdev = nd_region->provider_data;
+ struct virtio_pmem *vpmem = vdev->priv;
+ struct virtio_pmem_flush_work *flush;
+ int err;
- if (!child)
+ if (bio && bio->bi_iter.bi_sector != -1) {
+ flush = kmalloc_obj(*flush, GFP_NOIO);
+ if (!flush)
return -ENOMEM;
- bio_clone_blkg_association(child, bio);
- child->bi_iter.bi_sector = -1;
- bio_chain(child, bio);
- submit_bio(child);
- return 0;
+
+ INIT_WORK(&flush->work, virtio_pmem_flush_work);
+ flush->nd_region = nd_region;
+ flush->bio = bio;
+ queue_work(vpmem->flush_wq, &flush->work);
+ return NVDIMM_FLUSH_ASYNC;
}
- if (virtio_pmem_flush(nd_region))
+
+ err = virtio_pmem_flush(nd_region);
+ if (err > 0)
return -EIO;
- return 0;
+ return err;
};
EXPORT_SYMBOL_GPL(async_pmem_flush);
MODULE_DESCRIPTION("Virtio Persistent Memory Driver");
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 82ee1ddb3a445..30a51c365ce8b 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -241,8 +241,11 @@ static void pmem_submit_bio(struct bio *bio)
bio_end_io_acct(bio, start);
}
- if ((bio->bi_opf & REQ_FUA) && !bio->bi_status)
+ if ((bio->bi_opf & REQ_FUA) && !bio->bi_status) {
ret = nvdimm_flush(nd_region, bio);
+ if (ret == NVDIMM_FLUSH_ASYNC)
+ return;
+ }
if (ret)
bio->bi_status = errno_to_blk_status(ret);
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index 7cd2c2f0d3121..c540f1cff9250 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -1116,6 +1116,8 @@ int nvdimm_flush(struct nd_region *nd_region, struct bio *bio)
rc = generic_nvdimm_flush(nd_region);
else {
rc = nd_region->flush(nd_region, bio);
+ if (rc > 0)
+ return rc;
if (rc && rc != -ENOMEM)
rc = -EIO;
}
diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
index 77b1966619059..9cf822a6c0c38 100644
--- a/drivers/nvdimm/virtio_pmem.c
+++ b/drivers/nvdimm/virtio_pmem.c
@@ -67,10 +67,17 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
mutex_init(&vpmem->flush_lock);
vpmem->vdev = vdev;
vdev->priv = vpmem;
+ vpmem->flush_wq = alloc_ordered_workqueue("virtio-pmem-flush",
+ WQ_MEM_RECLAIM);
+ if (!vpmem->flush_wq) {
+ err = -ENOMEM;
+ goto out_err;
+ }
+
err = init_vq(vpmem);
if (err) {
dev_err(&vdev->dev, "failed to initialize virtio pmem vq's\n");
- goto out_err;
+ goto out_wq;
}
if (virtio_has_feature(vdev, VIRTIO_PMEM_F_SHMEM_REGION)) {
@@ -131,6 +138,8 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
nvdimm_bus_unregister(vpmem->nvdimm_bus);
out_vq:
vdev->config->del_vqs(vdev);
+out_wq:
+ destroy_workqueue(vpmem->flush_wq);
out_err:
return err;
}
@@ -138,14 +147,20 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
static void virtio_pmem_remove(struct virtio_device *vdev)
{
struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(&vdev->dev);
+ struct virtio_pmem *vpmem = vdev->priv;
nvdimm_bus_unregister(nvdimm_bus);
+ drain_workqueue(vpmem->flush_wq);
vdev->config->del_vqs(vdev);
virtio_reset_device(vdev);
+ destroy_workqueue(vpmem->flush_wq);
}
static int virtio_pmem_freeze(struct virtio_device *vdev)
{
+ struct virtio_pmem *vpmem = vdev->priv;
+
+ drain_workqueue(vpmem->flush_wq);
vdev->config->del_vqs(vdev);
virtio_reset_device(vdev);
diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
index f72cf17f9518f..e6dfc10ce0762 100644
--- a/drivers/nvdimm/virtio_pmem.h
+++ b/drivers/nvdimm/virtio_pmem.h
@@ -15,6 +15,7 @@
#include <linux/libnvdimm.h>
#include <linux/mutex.h>
#include <linux/spinlock.h>
+#include <linux/workqueue.h>
struct virtio_pmem_request {
struct virtio_pmem_req req;
@@ -39,6 +40,9 @@ struct virtio_pmem {
/* Serialize flush requests to the device. */
struct mutex flush_lock;
+ /* Complete asynchronous FUA flushes outside the submit path. */
+ struct workqueue_struct *flush_wq;
+
/* nvdimm bus registers virtio pmem device */
struct nvdimm_bus *nvdimm_bus;
struct nvdimm_bus_descriptor nd_desc;
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 28f086c4a1873..d929d83abf3be 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -126,6 +126,15 @@ struct nd_mapping_desc {
struct bio;
struct resource;
struct nd_region;
+
+/*
+ * Provider flush callback return values:
+ * 0: flush completed synchronously
+ * <0: flush failed
+ * >0: flush completion was queued and @bio will be completed later
+ */
+#define NVDIMM_FLUSH_ASYNC 1
+
struct nd_region_desc {
struct resource *res;
struct nd_mapping_desc *mapping;
--
2.52.0
^ permalink raw reply related
* [PATCH v7 05/12] nvdimm: virtio_pmem: use GFP_NOIO for flush requests
From: Li Chen @ 2026-06-30 9:23 UTC (permalink / raw)
To: Pankaj Gupta, Vishal Verma, Dave Jiang, Alison Schofield,
virtualization, nvdimm
Cc: linux-kernel, Li Chen
In-Reply-To: <20260630092338.2094628-1-me@linux.beauty>
virtio_pmem_flush() can run from pmem_submit_bio() while filesystem IO
is waiting on the flush completion. The request object allocation can
sleep, but it should not enter filesystem or block IO reclaim from this
flush path.
Use GFP_NOIO for the request allocation. The virtqueue descriptor
allocation still uses GFP_ATOMIC because it runs under pmem_lock.
Signed-off-by: Li Chen <me@linux.beauty>
---
Changes in v7:
- Keep GFP_NOIO for the virtio-pmem request allocation after removing the
child flush bio path.
Changes in v6:
- New patch; keep GFP_NOIO only for the virtio-pmem request allocation.
drivers/nvdimm/nd_virtio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index 8e16b7780be1a..a35044afddf34 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -61,7 +61,7 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
return -EIO;
}
- req_data = kmalloc_obj(*req_data);
+ req_data = kmalloc_obj(*req_data, GFP_NOIO);
if (!req_data)
return -ENOMEM;
--
2.52.0
^ permalink raw reply related
* [PATCH v7 06/12] nvdimm: virtio_pmem: always wake -ENOSPC waiters
From: Li Chen @ 2026-06-30 9:23 UTC (permalink / raw)
To: Pankaj Gupta, Vishal Verma, Dave Jiang, Alison Schofield,
virtualization, nvdimm
Cc: linux-kernel, Li Chen
In-Reply-To: <20260630092338.2094628-1-me@linux.beauty>
virtio_pmem_host_ack() reclaims virtqueue descriptors with
virtqueue_get_buf(). The -ENOSPC waiter wakeup is tied to completing the
returned token. If token completion is skipped for any reason, reclaimed
descriptors may not wake a waiter and the submitter may sleep forever
waiting for a free slot. Always wake one -ENOSPC waiter for each virtqueue
completion before touching the returned token.
Signed-off-by: Li Chen <me@linux.beauty>
---
v2->v3:
- Split out the waiter wakeup ordering change from READ_ONCE()/WRITE_ONCE()
updates (now patch 4/7), per Pankaj's suggestion.
v3->v4:
- Rebased onto v7.1-rc7 and renumbered after the flush error patches.
drivers/nvdimm/nd_virtio.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index a35044afddf34..fcb26a595d7c6 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -15,26 +15,33 @@ struct virtio_pmem_flush_work {
struct bio *bio;
};
+static void virtio_pmem_wake_one_waiter(struct virtio_pmem *vpmem)
+{
+ struct virtio_pmem_request *req_buf;
+
+ if (list_empty(&vpmem->req_list))
+ return;
+
+ req_buf = list_first_entry(&vpmem->req_list,
+ struct virtio_pmem_request, list);
+ req_buf->wq_buf_avail = true;
+ wake_up(&req_buf->wq_buf);
+ list_del(&req_buf->list);
+}
+
/* The interrupt handler */
void virtio_pmem_host_ack(struct virtqueue *vq)
{
struct virtio_pmem *vpmem = vq->vdev->priv;
- struct virtio_pmem_request *req_data, *req_buf;
+ struct virtio_pmem_request *req_data;
unsigned long flags;
unsigned int len;
spin_lock_irqsave(&vpmem->pmem_lock, flags);
while ((req_data = virtqueue_get_buf(vq, &len)) != NULL) {
+ virtio_pmem_wake_one_waiter(vpmem);
req_data->done = true;
wake_up(&req_data->host_acked);
-
- if (!list_empty(&vpmem->req_list)) {
- req_buf = list_first_entry(&vpmem->req_list,
- struct virtio_pmem_request, list);
- req_buf->wq_buf_avail = true;
- wake_up(&req_buf->wq_buf);
- list_del(&req_buf->list);
- }
}
spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
}
--
2.52.0
^ permalink raw reply related
* [PATCH v7 07/12] nvdimm: virtio_pmem: use READ_ONCE()/WRITE_ONCE() for wait flags
From: Li Chen @ 2026-06-30 9:23 UTC (permalink / raw)
To: Pankaj Gupta, Vishal Verma, Dave Jiang, Alison Schofield,
virtualization, nvdimm
Cc: linux-kernel, Li Chen
In-Reply-To: <20260630092338.2094628-1-me@linux.beauty>
Use READ_ONCE()/WRITE_ONCE() for the wait_event() flags (done and
wq_buf_avail). They are observed by waiters without pmem_lock, so make
the accesses explicit single loads/stores and avoid compiler
reordering/caching across the wait/wake paths.
Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
Signed-off-by: Li Chen <me@linux.beauty>
---
Changes in v7:
- Add Pankaj's Acked-by.
v2->v3:
- Split out READ_ONCE()/WRITE_ONCE() updates from patch 3/7 (no functional
change intended).
v3->v4:
- Rebased onto v7.1-rc7 and renumbered after the flush error patches.
drivers/nvdimm/nd_virtio.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index fcb26a595d7c6..8c0d4347938a1 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -24,9 +24,9 @@ static void virtio_pmem_wake_one_waiter(struct virtio_pmem *vpmem)
req_buf = list_first_entry(&vpmem->req_list,
struct virtio_pmem_request, list);
- req_buf->wq_buf_avail = true;
+ list_del_init(&req_buf->list);
+ WRITE_ONCE(req_buf->wq_buf_avail, true);
wake_up(&req_buf->wq_buf);
- list_del(&req_buf->list);
}
/* The interrupt handler */
@@ -40,7 +40,7 @@ void virtio_pmem_host_ack(struct virtqueue *vq)
spin_lock_irqsave(&vpmem->pmem_lock, flags);
while ((req_data = virtqueue_get_buf(vq, &len)) != NULL) {
virtio_pmem_wake_one_waiter(vpmem);
- req_data->done = true;
+ WRITE_ONCE(req_data->done, true);
wake_up(&req_data->host_acked);
}
spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
@@ -72,7 +72,7 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
if (!req_data)
return -ENOMEM;
- req_data->done = false;
+ WRITE_ONCE(req_data->done, false);
init_waitqueue_head(&req_data->host_acked);
init_waitqueue_head(&req_data->wq_buf);
INIT_LIST_HEAD(&req_data->list);
@@ -93,12 +93,12 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
GFP_ATOMIC)) == -ENOSPC) {
dev_info(&vdev->dev, "failed to send command to virtio pmem device, no free slots in the virtqueue\n");
- req_data->wq_buf_avail = false;
+ WRITE_ONCE(req_data->wq_buf_avail, false);
list_add_tail(&req_data->list, &vpmem->req_list);
spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
/* A host response results in "host_ack" getting called */
- wait_event(req_data->wq_buf, req_data->wq_buf_avail);
+ wait_event(req_data->wq_buf, READ_ONCE(req_data->wq_buf_avail));
spin_lock_irqsave(&vpmem->pmem_lock, flags);
}
err1 = virtqueue_kick(vpmem->req_vq);
@@ -112,7 +112,7 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
err = -EIO;
} else {
/* A host response results in "host_ack" getting called */
- wait_event(req_data->host_acked, req_data->done);
+ wait_event(req_data->host_acked, READ_ONCE(req_data->done));
err = le32_to_cpu(req_data->resp.ret);
}
--
2.52.0
^ permalink raw reply related
* [PATCH v7 08/12] nvdimm: virtio_pmem: refcount requests for token lifetime
From: Li Chen @ 2026-06-30 9:23 UTC (permalink / raw)
To: Pankaj Gupta, Vishal Verma, Dave Jiang, Alison Schofield,
virtualization, nvdimm
Cc: linux-kernel, stable, Li Chen
In-Reply-To: <20260630092338.2094628-1-me@linux.beauty>
KASAN reports slab-use-after-free in __wake_up_common():
BUG: KASAN: slab-use-after-free in __wake_up_common+0x114/0x160
Read of size 8 at addr ffff88810fdcb710 by task swapper/0/0
CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted
6.19.0-next-20260220-00006-g1eae5f204ec3 #4 PREEMPT(full)
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux
1.17.0-2-2 04/01/2014
Call Trace:
<IRQ>
dump_stack_lvl+0x6d/0xb0
print_report+0x170/0x4e2
? __pfx__raw_spin_lock_irqsave+0x10/0x10
? __virt_addr_valid+0x1dc/0x380
kasan_report+0xbc/0xf0
? __wake_up_common+0x114/0x160
? __wake_up_common+0x114/0x160
__wake_up_common+0x114/0x160
? __pfx__raw_spin_lock_irqsave+0x10/0x10
__wake_up+0x36/0x60
virtio_pmem_host_ack+0x11d/0x3b0
? sched_balance_domains+0x29f/0xb00
? __pfx_virtio_pmem_host_ack+0x10/0x10
? _raw_spin_lock_irqsave+0x98/0x100
? __pfx__raw_spin_lock_irqsave+0x10/0x10
vring_interrupt+0x1c9/0x5e0
? __pfx_vp_interrupt+0x10/0x10
vp_vring_interrupt+0x87/0x100
? __pfx_vp_interrupt+0x10/0x10
__handle_irq_event_percpu+0x17f/0x550
? __pfx__raw_spin_lock+0x10/0x10
handle_irq_event+0xab/0x1c0
handle_fasteoi_irq+0x276/0xae0
__common_interrupt+0x65/0x130
common_interrupt+0x78/0xa0
</IRQ>
virtio_pmem_host_ack() wakes a request that has already been freed by the
submitter.
This happens when the request token is still reachable via the virtqueue,
but virtio_pmem_flush() returns and frees it.
Fix the token lifetime by refcounting struct virtio_pmem_request.
virtio_pmem_flush() holds a submitter reference, and the virtqueue holds an
extra reference once the request is queued. The completion path drops the
virtqueue reference, and the submitter drops its reference before
returning.
Fixes: 6e84200c0a29 ("virtio-pmem: Add virtio pmem driver")
Cc: stable@vger.kernel.org
Signed-off-by: Li Chen <me@linux.beauty>
---
v2->v3:
- Add raw KASAN report to the patch description.
- Drop timestamps from the embedded report.
v3->v4:
- Rebased onto v7.1-rc7 and renumbered after the flush error patches.
drivers/nvdimm/nd_virtio.c | 34 +++++++++++++++++++++++++++++-----
drivers/nvdimm/virtio_pmem.h | 2 ++
2 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index 8c0d4347938a1..1cf53f75b1281 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -15,6 +15,14 @@ struct virtio_pmem_flush_work {
struct bio *bio;
};
+static void virtio_pmem_req_release(struct kref *kref)
+{
+ struct virtio_pmem_request *req;
+
+ req = container_of(kref, struct virtio_pmem_request, kref);
+ kfree(req);
+}
+
static void virtio_pmem_wake_one_waiter(struct virtio_pmem *vpmem)
{
struct virtio_pmem_request *req_buf;
@@ -42,6 +50,7 @@ void virtio_pmem_host_ack(struct virtqueue *vq)
virtio_pmem_wake_one_waiter(vpmem);
WRITE_ONCE(req_data->done, true);
wake_up(&req_data->host_acked);
+ kref_put(&req_data->kref, virtio_pmem_req_release);
}
spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
}
@@ -72,6 +81,7 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
if (!req_data)
return -ENOMEM;
+ kref_init(&req_data->kref);
WRITE_ONCE(req_data->done, false);
init_waitqueue_head(&req_data->host_acked);
init_waitqueue_head(&req_data->wq_buf);
@@ -89,10 +99,23 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
* to req_list and wait for host_ack to wake us up when free
* slots are available.
*/
- while ((err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req_data,
- GFP_ATOMIC)) == -ENOSPC) {
-
- dev_info(&vdev->dev, "failed to send command to virtio pmem device, no free slots in the virtqueue\n");
+ for (;;) {
+ err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req_data,
+ GFP_ATOMIC);
+ if (!err) {
+ /*
+ * Take the virtqueue reference while @pmem_lock is
+ * held so completion cannot run concurrently.
+ */
+ kref_get(&req_data->kref);
+ break;
+ }
+
+ if (err != -ENOSPC)
+ break;
+
+ dev_info_ratelimited(&vdev->dev,
+ "failed to send command to virtio pmem device, no free slots in the virtqueue\n");
WRITE_ONCE(req_data->wq_buf_avail, false);
list_add_tail(&req_data->list, &vpmem->req_list);
spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
@@ -101,6 +124,7 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
wait_event(req_data->wq_buf, READ_ONCE(req_data->wq_buf_avail));
spin_lock_irqsave(&vpmem->pmem_lock, flags);
}
+
err1 = virtqueue_kick(vpmem->req_vq);
spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
/*
@@ -116,7 +140,7 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
err = le32_to_cpu(req_data->resp.ret);
}
- kfree(req_data);
+ kref_put(&req_data->kref, virtio_pmem_req_release);
return err;
};
diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
index e6dfc10ce0762..3af92588bd9d1 100644
--- a/drivers/nvdimm/virtio_pmem.h
+++ b/drivers/nvdimm/virtio_pmem.h
@@ -12,12 +12,14 @@
#include <linux/module.h>
#include <uapi/linux/virtio_pmem.h>
+#include <linux/kref.h>
#include <linux/libnvdimm.h>
#include <linux/mutex.h>
#include <linux/spinlock.h>
#include <linux/workqueue.h>
struct virtio_pmem_request {
+ struct kref kref;
struct virtio_pmem_req req;
struct virtio_pmem_resp resp;
--
2.52.0
^ permalink raw reply related
* [PATCH v7 09/12] nvdimm: virtio_pmem: publish done with release/acquire
From: Li Chen @ 2026-06-30 9:23 UTC (permalink / raw)
To: Pankaj Gupta, Vishal Verma, Dave Jiang, Alison Schofield,
virtualization, nvdimm
Cc: linux-kernel, Li Chen
In-Reply-To: <20260630092338.2094628-1-me@linux.beauty>
virtio_pmem_host_ack() publishes the device response by setting done and
waking the submitter. The submitter reads resp.ret after wait_event()
observes done.
Use smp_store_release() on done and smp_load_acquire() in the wait
condition so the response read is ordered after completion.
Signed-off-by: Li Chen <me@linux.beauty>
---
Changes in v6:
- New patch.
drivers/nvdimm/nd_virtio.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index 1cf53f75b1281..e4e4284ae19e5 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -23,6 +23,19 @@ static void virtio_pmem_req_release(struct kref *kref)
kfree(req);
}
+static void virtio_pmem_signal_done(struct virtio_pmem_request *req)
+{
+ /* Pairs with smp_load_acquire() in virtio_pmem_req_done(). */
+ smp_store_release(&req->done, true);
+ wake_up(&req->host_acked);
+}
+
+static bool virtio_pmem_req_done(struct virtio_pmem_request *req)
+{
+ /* Pairs with smp_store_release() in virtio_pmem_signal_done(). */
+ return smp_load_acquire(&req->done);
+}
+
static void virtio_pmem_wake_one_waiter(struct virtio_pmem *vpmem)
{
struct virtio_pmem_request *req_buf;
@@ -48,8 +61,7 @@ void virtio_pmem_host_ack(struct virtqueue *vq)
spin_lock_irqsave(&vpmem->pmem_lock, flags);
while ((req_data = virtqueue_get_buf(vq, &len)) != NULL) {
virtio_pmem_wake_one_waiter(vpmem);
- WRITE_ONCE(req_data->done, true);
- wake_up(&req_data->host_acked);
+ virtio_pmem_signal_done(req_data);
kref_put(&req_data->kref, virtio_pmem_req_release);
}
spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
@@ -136,7 +148,8 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
err = -EIO;
} else {
/* A host response results in "host_ack" getting called */
- wait_event(req_data->host_acked, READ_ONCE(req_data->done));
+ wait_event(req_data->host_acked,
+ virtio_pmem_req_done(req_data));
err = le32_to_cpu(req_data->resp.ret);
}
--
2.52.0
^ permalink raw reply related
* [PATCH v7 10/12] nvdimm: virtio_pmem: isolate DMA request buffers
From: Li Chen @ 2026-06-30 9:23 UTC (permalink / raw)
To: Pankaj Gupta, Vishal Verma, Dave Jiang, Alison Schofield,
virtualization, nvdimm
Cc: linux-kernel, Li Chen
In-Reply-To: <20260630092338.2094628-1-me@linux.beauty>
The virtio-pmem request object stores wait queues, flags, and list
pointers next to buffers mapped for virtqueue DMA. The response buffer is
mapped DMA_FROM_DEVICE, so non-coherent DMA invalidation must not share a
cache line with CPU-owned fields.
Keep the request buffer outside the DMA-from-device group and wrap only
the response buffer with __dma_from_device_group_begin/end.
Signed-off-by: Li Chen <me@linux.beauty>
---
Changes in v6:
- New patch.
drivers/nvdimm/virtio_pmem.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
index 3af92588bd9d1..8843a8b965874 100644
--- a/drivers/nvdimm/virtio_pmem.h
+++ b/drivers/nvdimm/virtio_pmem.h
@@ -10,6 +10,7 @@
#ifndef _LINUX_VIRTIO_PMEM_H
#define _LINUX_VIRTIO_PMEM_H
+#include <linux/dma-mapping.h>
#include <linux/module.h>
#include <uapi/linux/virtio_pmem.h>
#include <linux/kref.h>
@@ -20,8 +21,6 @@
struct virtio_pmem_request {
struct kref kref;
- struct virtio_pmem_req req;
- struct virtio_pmem_resp resp;
/* Wait queue to process deferred work after ack from host */
wait_queue_head_t host_acked;
@@ -31,6 +30,11 @@ struct virtio_pmem_request {
wait_queue_head_t wq_buf;
bool wq_buf_avail;
struct list_head list;
+
+ struct virtio_pmem_req req;
+ __dma_from_device_group_begin(resp);
+ struct virtio_pmem_resp resp;
+ __dma_from_device_group_end(resp);
};
struct virtio_pmem {
--
2.52.0
^ permalink raw reply related
* [PATCH v7 11/12] nvdimm: virtio_pmem: converge broken virtqueue to -EIO
From: Li Chen @ 2026-06-30 9:23 UTC (permalink / raw)
To: Pankaj Gupta, Vishal Verma, Dave Jiang, Alison Schofield,
virtualization, nvdimm
Cc: linux-kernel, Li Chen
In-Reply-To: <20260630092338.2094628-1-me@linux.beauty>
dmesg reports virtqueue failure and device reset:
virtio_pmem virtio2: failed to send command to
virtio pmem device, no free slots in the virtqueue
virtio_pmem virtio2: virtio pmem device
needs a reset
virtio_pmem_flush() can wait for a free virtqueue descriptor (-ENOSPC).
It can also wait for host completion. If the request virtqueue breaks,
those waiters may never make progress. One example is notify failure from
virtqueue_kick().
Track a device-level broken state and converge the failure to -EIO. New
requests fail fast, -ENOSPC waiters are unlinked and woken, and the
currently submitted request is woken so its host_acked waiter can return
without waiting forever for host completion. Completed requests are forced
to report an error after the queue is marked broken.
Also serialize async parent-bio flush work against the broken state with
pmem_lock. That way remove and freeze either drain work queued before
virtio_pmem_mark_broken(), or later callers see nvdimm_flush() complete
the parent bio synchronously with -EIO instead of queuing work after the
drain point.
Do not detach unused buffers from an active virtqueue. Runtime
broken-queue handling only stops new submissions and wakes local waiters.
Removal resets the device first. It then drains request tokens. After
that, the device no longer owns the buffers when the virtqueue reference
is dropped.
Closes: https://lore.kernel.org/r/202512250116.ewtzlD0g-lkp@intel.com/
Signed-off-by: Li Chen <me@linux.beauty>
---
Changes in v7:
- Serialize async parent-bio flush work against broken-state publication so
remove/freeze cannot drain the workqueue before a racing FUA bio queues new
completion work.
Changes in v6:
- Wake the in-flight host-completion waiter when marking the queue broken.
- Track req_inflight and clear it on completion/drain paths.
- Return -EIO if the queue breaks before a host response is observed.
Changes in v5:
- Split broken marking from token draining.
- Do not call virtqueue_detach_unused_buf() on an active queue.
- Reset the device before draining tokens in remove().
- Do not let the host-completion wait return only because the device is
marked broken.
v2->v3:
- Add raw dmesg excerpt to the patch description.
- Drop timestamps from the embedded dmesg.
- Fold the CONFIG_VIRTIO_PMEM=m export fix into this patch.
v3->v4:
- Rebased onto v7.1-rc7 and renumbered after the flush error patches.
- Use kmalloc_obj(*req_data) at the allocation site to match current nvdimm
code.
drivers/nvdimm/nd_virtio.c | 126 +++++++++++++++++++++++++++++++----
drivers/nvdimm/virtio_pmem.c | 16 ++++-
drivers/nvdimm/virtio_pmem.h | 8 +++
3 files changed, 136 insertions(+), 14 deletions(-)
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index e4e4284ae19e5..a6820300cbe8f 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -36,6 +36,12 @@ static bool virtio_pmem_req_done(struct virtio_pmem_request *req)
return smp_load_acquire(&req->done);
}
+static void virtio_pmem_complete_err(struct virtio_pmem_request *req)
+{
+ req->resp.ret = cpu_to_le32(1);
+ virtio_pmem_signal_done(req);
+}
+
static void virtio_pmem_wake_one_waiter(struct virtio_pmem *vpmem)
{
struct virtio_pmem_request *req_buf;
@@ -50,6 +56,63 @@ static void virtio_pmem_wake_one_waiter(struct virtio_pmem *vpmem)
wake_up(&req_buf->wq_buf);
}
+static void virtio_pmem_wake_all_waiters(struct virtio_pmem *vpmem)
+{
+ struct virtio_pmem_request *req, *tmp;
+
+ list_for_each_entry_safe(req, tmp, &vpmem->req_list, list) {
+ list_del_init(&req->list);
+ WRITE_ONCE(req->wq_buf_avail, true);
+ wake_up(&req->wq_buf);
+ }
+}
+
+static void virtio_pmem_clear_inflight(struct virtio_pmem *vpmem,
+ struct virtio_pmem_request *req)
+{
+ if (vpmem->req_inflight == req)
+ vpmem->req_inflight = NULL;
+}
+
+static void virtio_pmem_wake_inflight(struct virtio_pmem *vpmem)
+{
+ struct virtio_pmem_request *req = vpmem->req_inflight;
+
+ if (req)
+ wake_up(&req->host_acked);
+}
+
+void virtio_pmem_mark_broken(struct virtio_pmem *vpmem)
+{
+ if (!READ_ONCE(vpmem->broken)) {
+ WRITE_ONCE(vpmem->broken, true);
+ dev_err_once(&vpmem->vdev->dev, "virtqueue is broken\n");
+ }
+
+ virtio_pmem_wake_inflight(vpmem);
+ virtio_pmem_wake_all_waiters(vpmem);
+}
+EXPORT_SYMBOL_GPL(virtio_pmem_mark_broken);
+
+void virtio_pmem_drain(struct virtio_pmem *vpmem)
+{
+ struct virtio_pmem_request *req;
+ unsigned int len;
+
+ while ((req = virtqueue_get_buf(vpmem->req_vq, &len)) != NULL) {
+ virtio_pmem_clear_inflight(vpmem, req);
+ virtio_pmem_complete_err(req);
+ kref_put(&req->kref, virtio_pmem_req_release);
+ }
+
+ while ((req = virtqueue_detach_unused_buf(vpmem->req_vq)) != NULL) {
+ virtio_pmem_clear_inflight(vpmem, req);
+ virtio_pmem_complete_err(req);
+ kref_put(&req->kref, virtio_pmem_req_release);
+ }
+}
+EXPORT_SYMBOL_GPL(virtio_pmem_drain);
+
/* The interrupt handler */
void virtio_pmem_host_ack(struct virtqueue *vq)
{
@@ -60,8 +123,12 @@ void virtio_pmem_host_ack(struct virtqueue *vq)
spin_lock_irqsave(&vpmem->pmem_lock, flags);
while ((req_data = virtqueue_get_buf(vq, &len)) != NULL) {
+ virtio_pmem_clear_inflight(vpmem, req_data);
virtio_pmem_wake_one_waiter(vpmem);
- virtio_pmem_signal_done(req_data);
+ if (READ_ONCE(vpmem->broken))
+ virtio_pmem_complete_err(req_data);
+ else
+ virtio_pmem_signal_done(req_data);
kref_put(&req_data->kref, virtio_pmem_req_release);
}
spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
@@ -89,6 +156,9 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
return -EIO;
}
+ if (READ_ONCE(vpmem->broken))
+ return -EIO;
+
req_data = kmalloc_obj(*req_data, GFP_NOIO);
if (!req_data)
return -ENOMEM;
@@ -105,13 +175,18 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
sgs[1] = &ret;
spin_lock_irqsave(&vpmem->pmem_lock, flags);
- /*
- * If virtqueue_add_sgs returns -ENOSPC then req_vq virtual
- * queue does not have free descriptor. We add the request
- * to req_list and wait for host_ack to wake us up when free
- * slots are available.
- */
+ /*
+ * If virtqueue_add_sgs returns -ENOSPC then req_vq virtual
+ * queue does not have free descriptor. We add the request
+ * to req_list and wait for host_ack to wake us up when free
+ * slots are available.
+ */
for (;;) {
+ if (READ_ONCE(vpmem->broken)) {
+ err = -EIO;
+ break;
+ }
+
err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req_data,
GFP_ATOMIC);
if (!err) {
@@ -120,6 +195,7 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
* held so completion cannot run concurrently.
*/
kref_get(&req_data->kref);
+ vpmem->req_inflight = req_data;
break;
}
@@ -133,24 +209,41 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
/* A host response results in "host_ack" getting called */
- wait_event(req_data->wq_buf, READ_ONCE(req_data->wq_buf_avail));
+ wait_event(req_data->wq_buf,
+ READ_ONCE(req_data->wq_buf_avail) ||
+ READ_ONCE(vpmem->broken));
spin_lock_irqsave(&vpmem->pmem_lock, flags);
+
+ if (READ_ONCE(vpmem->broken))
+ break;
}
- err1 = virtqueue_kick(vpmem->req_vq);
+ if (err == -EIO || virtqueue_is_broken(vpmem->req_vq))
+ virtio_pmem_mark_broken(vpmem);
+
+ err1 = true;
+ if (!err && !READ_ONCE(vpmem->broken)) {
+ err1 = virtqueue_kick(vpmem->req_vq);
+ if (!err1)
+ virtio_pmem_mark_broken(vpmem);
+ }
spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
/*
* virtqueue_add_sgs failed with error different than -ENOSPC, we can't
* do anything about that.
*/
- if (err || !err1) {
+ if (READ_ONCE(vpmem->broken) || err || !err1) {
dev_info(&vdev->dev, "failed to send command to virtio pmem device\n");
err = -EIO;
} else {
/* A host response results in "host_ack" getting called */
wait_event(req_data->host_acked,
- virtio_pmem_req_done(req_data));
- err = le32_to_cpu(req_data->resp.ret);
+ virtio_pmem_req_done(req_data) ||
+ READ_ONCE(vpmem->broken));
+ if (virtio_pmem_req_done(req_data))
+ err = le32_to_cpu(req_data->resp.ret);
+ else
+ err = -EIO;
}
kref_put(&req_data->kref, virtio_pmem_req_release);
@@ -178,6 +271,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
struct virtio_device *vdev = nd_region->provider_data;
struct virtio_pmem *vpmem = vdev->priv;
struct virtio_pmem_flush_work *flush;
+ unsigned long flags;
int err;
if (bio && bio->bi_iter.bi_sector != -1) {
@@ -188,7 +282,15 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
INIT_WORK(&flush->work, virtio_pmem_flush_work);
flush->nd_region = nd_region;
flush->bio = bio;
+
+ spin_lock_irqsave(&vpmem->pmem_lock, flags);
+ if (READ_ONCE(vpmem->broken)) {
+ spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
+ kfree(flush);
+ return -EIO;
+ }
queue_work(vpmem->flush_wq, &flush->work);
+ spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
return NVDIMM_FLUSH_ASYNC;
}
diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
index 9cf822a6c0c38..36664a5ea25e3 100644
--- a/drivers/nvdimm/virtio_pmem.c
+++ b/drivers/nvdimm/virtio_pmem.c
@@ -25,6 +25,8 @@ static int init_vq(struct virtio_pmem *vpmem)
spin_lock_init(&vpmem->pmem_lock);
INIT_LIST_HEAD(&vpmem->req_list);
+ vpmem->req_inflight = NULL;
+ WRITE_ONCE(vpmem->broken, false);
return 0;
};
@@ -148,11 +150,21 @@ static void virtio_pmem_remove(struct virtio_device *vdev)
{
struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(&vdev->dev);
struct virtio_pmem *vpmem = vdev->priv;
+ unsigned long flags;
+
+ spin_lock_irqsave(&vpmem->pmem_lock, flags);
+ virtio_pmem_mark_broken(vpmem);
+ spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
- nvdimm_bus_unregister(nvdimm_bus);
drain_workqueue(vpmem->flush_wq);
- vdev->config->del_vqs(vdev);
virtio_reset_device(vdev);
+
+ spin_lock_irqsave(&vpmem->pmem_lock, flags);
+ virtio_pmem_drain(vpmem);
+ spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
+
+ nvdimm_bus_unregister(nvdimm_bus);
+ vdev->config->del_vqs(vdev);
destroy_workqueue(vpmem->flush_wq);
}
diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
index 8843a8b965874..0b90777d7658b 100644
--- a/drivers/nvdimm/virtio_pmem.h
+++ b/drivers/nvdimm/virtio_pmem.h
@@ -56,6 +56,12 @@ struct virtio_pmem {
/* List to store deferred work if virtqueue is full */
struct list_head req_list;
+ /* Request currently owned by the virtqueue. */
+ struct virtio_pmem_request *req_inflight;
+
+ /* Fail fast and wake waiters if the request virtqueue is broken. */
+ bool broken;
+
/* Synchronize virtqueue data */
spinlock_t pmem_lock;
@@ -65,5 +71,7 @@ struct virtio_pmem {
};
void virtio_pmem_host_ack(struct virtqueue *vq);
+void virtio_pmem_mark_broken(struct virtio_pmem *vpmem);
+void virtio_pmem_drain(struct virtio_pmem *vpmem);
int async_pmem_flush(struct nd_region *nd_region, struct bio *bio);
#endif
--
2.52.0
^ permalink raw reply related
* [PATCH v7 12/12] nvdimm: virtio_pmem: drain requests in freeze
From: Li Chen @ 2026-06-30 9:23 UTC (permalink / raw)
To: Pankaj Gupta, Vishal Verma, Dave Jiang, Alison Schofield,
virtualization, nvdimm
Cc: linux-kernel, Li Chen
In-Reply-To: <20260630092338.2094628-1-me@linux.beauty>
virtio_pmem_freeze() currently deletes virtqueues and resets the device
without waking threads waiting for a virtqueue descriptor or a host
completion.
Mark the request virtqueue broken before reset. This makes new submissions
fail fast and lets -ENOSPC waiters leave the wait list. Reset the device
before draining used and unused request tokens, then delete the virtqueues.
This wakes waiters with -EIO. It also keeps the detach call on a quiesced
device.
Clear req_vq after del_vqs(). Make drain tolerate a NULL queue so remove
after freeze does not dereference a stale virtqueue pointer. Also make
virtio_pmem_flush() stop checking req_vq once the broken state is visible.
A waiter woken by freeze/remove can resume after del_vqs() has cleared
req_vq.
Signed-off-by: Li Chen <me@linux.beauty>
---
Changes in v7:
- Stop checking req_vq once the broken state is visible, so a waiter woken
by freeze/remove does not dereference req_vq after del_vqs() clears it.
Changes in v6:
- Clear req_vq after del_vqs() and make drain tolerate a NULL queue.
Changes in v5:
- Reset the device before draining used and unused request tokens.
- Use the split broken-marking and post-reset drain helpers.
v2->v3:
- No change.
v3->v4:
- Rebased onto v7.1-rc7 and renumbered after the flush error patches.
drivers/nvdimm/nd_virtio.c | 5 +++++
drivers/nvdimm/virtio_pmem.c | 34 +++++++++++++++++++++++++++++-----
2 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index a6820300cbe8f..3b8be79a20a0f 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -99,6 +99,9 @@ void virtio_pmem_drain(struct virtio_pmem *vpmem)
struct virtio_pmem_request *req;
unsigned int len;
+ if (!vpmem->req_vq)
+ return;
+
while ((req = virtqueue_get_buf(vpmem->req_vq, &len)) != NULL) {
virtio_pmem_clear_inflight(vpmem, req);
virtio_pmem_complete_err(req);
@@ -218,6 +221,8 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
break;
}
+ if (READ_ONCE(vpmem->broken))
+ err = -EIO;
if (err == -EIO || virtqueue_is_broken(vpmem->req_vq))
virtio_pmem_mark_broken(vpmem);
diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
index 36664a5ea25e3..7ee3fb1779f73 100644
--- a/drivers/nvdimm/virtio_pmem.c
+++ b/drivers/nvdimm/virtio_pmem.c
@@ -17,11 +17,16 @@ static struct virtio_device_id id_table[] = {
/* Initialize virt queue */
static int init_vq(struct virtio_pmem *vpmem)
{
+ int err;
+
/* single vq */
vpmem->req_vq = virtio_find_single_vq(vpmem->vdev,
virtio_pmem_host_ack, "flush_queue");
- if (IS_ERR(vpmem->req_vq))
- return PTR_ERR(vpmem->req_vq);
+ if (IS_ERR(vpmem->req_vq)) {
+ err = PTR_ERR(vpmem->req_vq);
+ vpmem->req_vq = NULL;
+ return err;
+ }
spin_lock_init(&vpmem->pmem_lock);
INIT_LIST_HEAD(&vpmem->req_list);
@@ -31,6 +36,15 @@ static int init_vq(struct virtio_pmem *vpmem)
return 0;
};
+static void virtio_pmem_del_vqs(struct virtio_pmem *vpmem)
+{
+ if (!vpmem->req_vq)
+ return;
+
+ vpmem->vdev->config->del_vqs(vpmem->vdev);
+ vpmem->req_vq = NULL;
+}
+
static int virtio_pmem_validate(struct virtio_device *vdev)
{
struct virtio_shm_region shm_reg;
@@ -139,7 +153,7 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
virtio_reset_device(vdev);
nvdimm_bus_unregister(vpmem->nvdimm_bus);
out_vq:
- vdev->config->del_vqs(vdev);
+ virtio_pmem_del_vqs(vpmem);
out_wq:
destroy_workqueue(vpmem->flush_wq);
out_err:
@@ -164,18 +178,28 @@ static void virtio_pmem_remove(struct virtio_device *vdev)
spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
nvdimm_bus_unregister(nvdimm_bus);
- vdev->config->del_vqs(vdev);
+ virtio_pmem_del_vqs(vpmem);
destroy_workqueue(vpmem->flush_wq);
}
static int virtio_pmem_freeze(struct virtio_device *vdev)
{
struct virtio_pmem *vpmem = vdev->priv;
+ unsigned long flags;
+
+ spin_lock_irqsave(&vpmem->pmem_lock, flags);
+ virtio_pmem_mark_broken(vpmem);
+ spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
drain_workqueue(vpmem->flush_wq);
- vdev->config->del_vqs(vdev);
virtio_reset_device(vdev);
+ spin_lock_irqsave(&vpmem->pmem_lock, flags);
+ virtio_pmem_drain(vpmem);
+ spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
+
+ virtio_pmem_del_vqs(vpmem);
+
return 0;
}
--
2.52.0
^ permalink raw reply related
* Re: [PATCH net-next v4] vsock/virtio: rewrite MSG_ZEROCOPY flag handling
From: Stefano Garzarella @ 2026-06-30 9:41 UTC (permalink / raw)
To: Arseniy Krasnov
Cc: Stefan Hajnoczi, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Michael S. Tsirkin, Jason Wang, Bobby Eshleman,
Xuan Zhuo, Eugenio Pérez, Simon Horman, kvm, virtualization,
netdev, linux-kernel, oxffffaa, rulkc
In-Reply-To: <20260628182052.951760-1-avkrasnov@rulkc.org>
On Sun, Jun 28, 2026 at 09:20:52PM +0300, Arseniy Krasnov wrote:
>Logically it was based on TCP implementation, so to make further support
>easier, rewrite it in the TCP way (like in 'tcp_sendmsg_locked()'). By
>this way, patch also adds handling case when 'msg_ubuf' is already set.
Thanks for this!
IIUC the result will be similar of commit eb315a7d1396 ("tcp: support
externally provided ubufs") for tcp. Maybe I would have added it to the
commit description, anyway the patch LGTM:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>Signed-off-by: Arseniy Krasnov <avkrasnov@rulkc.org>
>---
> Changelog v1->v2:
> * Rebase on last 'net-next'. Don't need 'skb_zcopy_set()' now - it was
> already added.
> Changelog v2->v3:
> * Update commit message.
> * Remove one empty line.
> Changelog v3->v4:
> * Update commit message.
>
> net/vmw_vsock/virtio_transport_common.c | 47 ++++++++++++-------------
> 1 file changed, 22 insertions(+), 25 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 09475007165b..41c2a0b82a8e 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -328,38 +328,35 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> if (pkt_len == 0 && info->op == VIRTIO_VSOCK_OP_RW)
> return pkt_len;
>
>- if (info->msg) {
>- /* If zerocopy is not enabled by 'setsockopt()', we behave as
>- * there is no MSG_ZEROCOPY flag set.
>+ if (info->msg && (info->msg->msg_flags & MSG_ZEROCOPY)) {
>+ /* If 'info->msg' is not NULL, this is only VIRTIO_VSOCK_OP_RW.
>+ * 'MSG_ZEROCOPY' flag handling here is based on the same flag
>+ * handling from 'tcp_sendmsg_locked()'.
> */
>- if (!sock_flag(sk_vsock(vsk), SOCK_ZEROCOPY))
>- info->msg->msg_flags &= ~MSG_ZEROCOPY;
>+ if (info->msg->msg_ubuf) {
>+ uarg = info->msg->msg_ubuf;
>+ can_zcopy = virtio_transport_can_zcopy(t_ops, info, pkt_len);
>+ } else if (sock_flag(sk_vsock(vsk), SOCK_ZEROCOPY)) {
>+ uarg = msg_zerocopy_realloc(sk_vsock(vsk), pkt_len,
>+ NULL, false);
>+ if (!uarg) {
>+ virtio_transport_put_credit(vvs, pkt_len);
>+ return -ENOMEM;
>+ }
>
>- if (info->msg->msg_flags & MSG_ZEROCOPY)
> can_zcopy = virtio_transport_can_zcopy(t_ops, info, pkt_len);
>+ if (!can_zcopy)
>+ uarg_to_msgzc(uarg)->zerocopy = 0;
>
>+ have_uref = true;
>+ }
>+
>+ /* 'can_zcopy' means that this transmission will be
>+ * in zerocopy way (e.g. using 'frags' array).
>+ */
> if (can_zcopy)
> max_skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE,
> (MAX_SKB_FRAGS * PAGE_SIZE));
>-
>- if (info->msg->msg_flags & MSG_ZEROCOPY &&
>- info->op == VIRTIO_VSOCK_OP_RW) {
>- uarg = info->msg->msg_ubuf;
>-
>- if (!uarg) {
>- uarg = msg_zerocopy_realloc(sk_vsock(vsk),
>- pkt_len, NULL, false);
>- if (!uarg) {
>- virtio_transport_put_credit(vvs, pkt_len);
>- return -ENOMEM;
>- }
>-
>- if (!can_zcopy)
>- uarg_to_msgzc(uarg)->zerocopy = 0;
>-
>- have_uref = true;
>- }
>- }
> }
>
> rest_len = pkt_len;
>--
>2.25.1
>
^ permalink raw reply
* Re: [PATCH v7 00/12] nvdimm: virtio_pmem: fix flush/request failure paths
From: Pankaj Gupta @ 2026-06-30 9:47 UTC (permalink / raw)
To: Li Chen
Cc: Vishal Verma, Dave Jiang, Alison Schofield, virtualization,
nvdimm, linux-kernel, Michael S . Tsirkin, Dan Williams
In-Reply-To: <20260630092338.2094628-1-me@linux.beauty>
+CC Dan's correct email address and MST's email.
> Hi,
>
> This series started as a virtio-pmem request lifetime and broken virtqueue
> fix, but the rerolls have picked up several related flush-path fixes found
> during local testing and review. Since the series is now broader than the
> original lifetime bug, this cover letter calls out where the patches came
> from.
>
> The nvdimm flush helper maps provider flush failures to -EIO. That should
> remain the default for provider/backend failures because host-side errors are
> still best reported as generic I/O errors to the guest. However, virtio-pmem
> may also fail a guest-local flush request allocation with -ENOMEM before any
> request is submitted to the host. Reporting that resource failure as -EIO
> makes memory pressure look like media failure.
>
> The raw failure seen in the local mkfs sanity test was:
>
> wipefs: /dev/pmem0: cannot flush modified buffers: Input/output error
> mkfs.ext4: Input/output error while writing out and closing file system
> nd_region region0: dbg: nvdimm_flush rc=-5
>
> Patch 1 comes from that local failure, with the error policy narrowed after
> Pankaj pointed out that host/backend provider errors should not all be exposed
> directly to the guest. It now preserves only -ENOMEM and keeps other provider
> flush failures mapped to -EIO.
>
> Patches 2 and 3 come from review of the pmem flush path. Patch 2 keeps a
> failed REQ_PREFLUSH from being overwritten after data copy, and patch 3 is the
> dataless-bio guard added after the Sashiko review. Patch 4 comes from the
> local child flush bio allocation failure, but v7 reworks the v6 synchronous
> FUA approach after Pankaj noted that the old child flush bio path completed
> asynchronously. This version removes the child bio while keeping parent bio
> completion asynchronous: the provider returns NVDIMM_FLUSH_ASYNC, queues
> ordered WQ_MEM_RECLAIM work, and completes the parent bio after
> virtio_pmem_flush() finishes. Patch 5 is the remaining allocation-policy
> follow-up for the actual virtio-pmem flush request object, not for a child
> bio.
>
> Patches 6 and 7 are the older waiter fixes. Patch 6 wakes one -ENOSPC waiter
> for each reclaimed used buffer, and patch 7 makes the wait flags explicit
> READ_ONCE()/WRITE_ONCE() accesses. Pankaj asked for those changes to be split
> across patches, and patch 7 carries his Acked-by.
>
> Patch 8 is the original KASAN use-after-free fix for the request token
> lifetime. Patches 9 and 10 are follow-up hardening in the same completion
> path: order response publication before the submitter reads resp.ret, and keep
> the DMA_FROM_DEVICE response buffer away from CPU-owned request fields. Patch
> 11 addresses the broken virtqueue / notify failure path reported by LKP and
> reproduced locally with fault injection. It also serializes async parent-bio
> flush work against broken-state publication, so remove/freeze cannot drain the
> workqueue before a racing FUA bio queues new completion work. Patch 12 handles
> teardown: it drains requests across freeze/remove and also addresses the
> Sashiko-reported req_vq-after-free/NULL-deref class by clearing req_vq after
> del_vqs() and making the drain helper tolerate a NULL queue. It also stops the
> submit path from checking req_vq after the broken state is visible.
>
> The original repros were on QEMU x86_64 with a virtio-pmem device exported
> as /dev/pmem0. For this v7 reroll, the series applies to v7.1-rc7.
>
> Thanks,
> Li Chen
>
> Changelog:
> v6->v7:
> - Address Pankaj's feedback on nvdimm_flush() error policy.
> - Preserve only -ENOMEM from provider flush callbacks and continue to map
> other provider/backend failures to -EIO.
> - Address Pankaj's feedback on the FUA flush behavior: replace the v6
> synchronous FUA path with provider-owned asynchronous parent bio completion.
> - Add NVDIMM_FLUSH_ASYNC and use ordered WQ_MEM_RECLAIM work to run
> virtio_pmem_flush() and complete the parent bio after the host flush.
> - Keep GFP_NOIO for the virtio-pmem request allocation, but no longer describe
> it as a child bio allocation fix.
> - Add Pankaj's Acked-by on the READ_ONCE()/WRITE_ONCE() patch.
> - Serialize async parent-bio flush work against broken-state publication in
> the broken-virtqueue patch, so remove/freeze cannot drain the workqueue
> before a racing FUA bio queues new completion work.
> - Fold the Sashiko-reported req_vq NULL-deref fix into the freeze/remove
> drain patch.
> - Update commit messages and this cover letter to describe patch origins.
> v5->v6:
> - Address Sashiko review feedback:
> - Add a data-loop guard for dataless bios in pmem_submit_bio().
> - Replace the child flush bio allocation with synchronous FUA flushing.
> - Keep GFP_NOIO only for the virtio-pmem request allocation.
> - Publish request completion with release/acquire ordering.
> - Isolate the DMA_FROM_DEVICE response buffer from CPU-owned fields.
> - Wake the in-flight host-completion waiter when marking the queue broken.
> - Clear req_vq after del_vqs() and make drain tolerate a NULL queue.
> v4->v5:
> - Address review feedback about REQ_PREFLUSH ordering and active virtqueue
> detach.
> - Add 2/8 so a failed REQ_PREFLUSH fails the bio before any data copy, and
> make REQ_PREFLUSH use a synchronous provider flush instead of a deferred
> child bio.
> - Rework broken-queue handling so runtime failure marking only stops new
> submissions and wakes local -ENOSPC waiters; used/unused token draining is
> done after device reset in remove() and freeze().
> - Remove the broken-state shortcut from the host-completion wait so the
> submitter never reads an uninitialized response field.
> - Keep the raw broken-virtqueue dmesg in 7/8 while updating the teardown
> rationale.
> - Renumber the old virtio-pmem fixes after the new pmem PREFLUSH patch.
> v3->v4:
> - Rebased the series onto v7.1-rc7 so it applies cleanly to Linux 7.1-rc7.
> - Update the allocation site in 6/7 from kmalloc(sizeof(*req_data),
> GFP_KERNEL) to kmalloc_obj(*req_data) to match current nvdimm code.
> - Add 1/7 to preserve provider flush callback errors in nvdimm_flush().
> - Include the GFP_NOIO child flush bio allocation fix as 2/7.
> - Renumber the old request lifetime and broken virtqueue fixes after the two
> new flush error patches.
> v2->v3:
> - Split patch 1 as suggested by Pankaj Gupta: keep the waiter wakeup
> ordering change in 1/5 and move READ_ONCE()/WRITE_ONCE() updates to
> 2/5 (no functional change intended).
> - Add log report to commit msg.
> - Fold the export fix into 4/5 to keep the series bisectable when
> CONFIG_VIRTIO_PMEM=m.
> v1->v2:
> - Add the export patch to fix compile issue.
>
> Links:
> v6: https://lore.kernel.org/all/20260621130246.2973254-1-me@linux.beauty/
> v5: https://lore.kernel.org/all/20260617122442.2118957-1-me@linux.beauty/
> v4: https://lore.kernel.org/all/20260609120726.1714780-1-me@linux.beauty/
> v3: https://lore.kernel.org/all/20260226025712.2236279-1-me@linux.beauty/#t
> v2: https://lore.kernel.org/all/20251225042915.334117-1-me@linux.beauty/
> v1: https://www.spinics.net/lists/kernel/msg5974818.html
>
> Li Chen (12):
> nvdimm: preserve flush callback -ENOMEM
> nvdimm: pmem: keep PREFLUSH before data writes
> nvdimm: pmem: guard data loop for dataless bios
> nvdimm: virtio_pmem: stop allocating child flush bio
> nvdimm: virtio_pmem: use GFP_NOIO for flush requests
> nvdimm: virtio_pmem: always wake -ENOSPC waiters
> nvdimm: virtio_pmem: use READ_ONCE()/WRITE_ONCE() for wait flags
> nvdimm: virtio_pmem: refcount requests for token lifetime
> nvdimm: virtio_pmem: publish done with release/acquire
> nvdimm: virtio_pmem: isolate DMA request buffers
> nvdimm: virtio_pmem: converge broken virtqueue to -EIO
> nvdimm: virtio_pmem: drain requests in freeze
>
> drivers/nvdimm/nd_virtio.c | 265 +++++++++++++++++++++++++++++------
> drivers/nvdimm/pmem.c | 51 ++++---
> drivers/nvdimm/region_devs.c | 5 +-
> drivers/nvdimm/virtio_pmem.c | 65 ++++++++-
> drivers/nvdimm/virtio_pmem.h | 22 ++-
> include/linux/libnvdimm.h | 9 ++
> 6 files changed, 343 insertions(+), 74 deletions(-)
>
> --
> 2.52.0
^ permalink raw reply
* Re: [PATCH net 1/2] vsock/virtio: collapse receive queue under memory pressure
From: Paolo Abeni @ 2026-06-30 9:53 UTC (permalink / raw)
To: Stefano Garzarella, netdev
Cc: Jason Wang, Jakub Kicinski, Michael S. Tsirkin, kvm,
virtualization, Xuan Zhuo, Eric Dumazet, Simon Horman,
linux-kernel, Stefan Hajnoczi, David S. Miller,
Eugenio Pérez, stable, Brien Oberstein
In-Reply-To: <20260626134823.206676-2-sgarzare@redhat.com>
On 6/26/26 3:48 PM, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
>
> When many small packets accumulate in the receive queue, the skb overhead
> can exceed buf_alloc even while the payload is within bounds. This causes
> virtio_transport_inc_rx_pkt() to reject packets, leading to connection
> resets during large transfers under backpressure.
>
> The issue was reported by Brien, who has a reproducer, but it is also
> easily reproducible with iperf-vsock [1] using a small packet size:
>
> iperf3 --vsock -c $CID -l 129
>
> which fails immediately without this patch but with commit 059b7dbd20a6
> ("vsock/virtio: fix potential unbounded skb queue").
>
> Inspired by TCP's tcp_collapse() which solves a similar problem, add
> virtio_transport_collapse_rx_queue() that walks the receive queue and
> re-copies data into compact linear skbs to reduce the overhead.
>
> The collapse is triggered from virtio_transport_recv_enqueue() when
> virtio_transport_inc_rx_pkt() fails. A pre-scan counts the eligible bytes
> to size each allocation precisely, avoiding waste for isolated small
> packets. Partially consumed skbs are kept as-is to preserve
> buf_used/fwd_cnt accounting, EOM-marked skbs to maintain SEQPACKET
> message boundaries, and skbs already larger than the collapse target
> because they already have a good data-to-overhead ratio.
>
> [1] https://github.com/stefano-garzarella/iperf-vsock
>
> Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
> Cc: stable@vger.kernel.org
> Reported-by: Brien Oberstein <brienpub@gmail.com>
> Closes: https://lore.kernel.org/netdev/618701dd023e$063de350$12b9a9f0$@gmail.com/
> Tested-by: Brien Oberstein <brienpub@gmail.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> net/vmw_vsock/virtio_transport_common.c | 148 +++++++++++++++++++++++-
> 1 file changed, 146 insertions(+), 2 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 09475007165b..304ea424995d 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -420,6 +420,137 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> return ret;
> }
>
> +static bool virtio_transport_can_collapse(struct sk_buff *skb,
> + unsigned int size)
Why passing a `size` argument here? AFAICS the actual argument is always
a constant and IMHO rightfully so.
> +{
> + /* skbs that are partially consumed, mark a SEQPACKET message boundary,
> + * or are already large enough should not be collapsed: they either
> + * need special accounting, carry protocol state, or already have a
> + * good data-to-overhead ratio.
> + */
> + if (VIRTIO_VSOCK_SKB_CB(skb)->offset)
> + return false;
> + if (le32_to_cpu(virtio_vsock_hdr(skb)->flags) & VIRTIO_VSOCK_SEQ_EOM)
> + return false;
> + if (skb->len >= size)
> + return false;
> + return true;
> +}
> +
> +/* Iterate through the packets in the queue starting from the current skb to
> + * count the number of bytes we can collapse.
> + */
> +static unsigned int
> +virtio_transport_collapse_size(struct sk_buff *skb,
> + struct sk_buff_head *queue,
> + unsigned int max_size)
> +{
> + unsigned int target = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
> +
> + while ((skb = skb_peek_next(skb, queue)) &&
> + virtio_transport_can_collapse(skb, max_size)) {
> + unsigned int len = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
> +
> + if (len > max_size - target)
> + return target;
> +
> + target += len;
> + }
> +
> + return target;
> +}
> +
> +/* Called under lock_sock when skb overhead exceeds the budget. */
> +static void virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs)
> +{
> + /* Use the same linear allocation threshold as virtio_vsock_alloc_skb()
> + * to avoid adding pressure on the page allocator.
> + */
> + unsigned int collapse_max = SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM,
> + PAGE_ALLOC_COSTLY_ORDER);
> + struct sk_buff *skb, *next_skb, *new_skb = NULL;
> + struct sk_buff_head new_queue;
> +
> + __skb_queue_head_init(&new_queue);
> +
> + skb_queue_walk_safe(&vvs->rx_queue, skb, next_skb) {
If the queue is relevantly big, walking all of it may take a significant
amount of time/cache misses and causes traffic burstines. I think you
could add an additional stop condition, i.e. when the current queue size
is below a reasonable threshold (allowing the current packet to be
inserted plus some more slack).
/P
> + struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> + u32 src_off = VIRTIO_VSOCK_SKB_CB(skb)->offset;
> + u32 src_len = skb->len - src_off;
> + bool keep = false;
> +
> + if (!virtio_transport_can_collapse(skb, collapse_max)) {
Minor nit, possibly something alike the following lead to more
compact/more readable code:
keep = !virtio_transport_can_collapse(skb, collapse_max);
if (keep) {
> + /* Finalize pending collapsed skb to preserve packet
> + * ordering.
> + */
> + if (new_skb) {
> + __skb_queue_tail(&new_queue, new_skb);
> + new_skb = NULL;
> + }
> + keep = true;
> + goto next;
> + }
> +
> + /* Finalize if this packet won't fit in the remaining tailroom,
> + * so we can allocate a right-sized new_skb.
> + */
> + if (new_skb && src_len > skb_tailroom(new_skb)) {
> + __skb_queue_tail(&new_queue, new_skb);
> + new_skb = NULL;
Possibly introduce an helper for the above 2 statements?
/P
^ permalink raw reply
* Re: [PATCH 08/13] mm: introduce vma_get_page_prot() and use it
From: Jani Nikula @ 2026-06-30 10:23 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton
Cc: Thomas Bogendoerfer, Madhavan Srinivasan, Michael Ellerman,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Lucas Stach, Inki Dae, Seung-Woo Kim,
Kyungmin Park, Krzysztof Kozlowski, Peter Griffin,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Rob Clark,
Dmitry Baryshkov, Lyude Paul, Danilo Krummrich, Tomi Valkeinen,
Sandy Huang, Heiko Stübner, Andy Yan, Thierry Reding,
Mikko Perttunen, Jonathan Hunter, Gerd Hoffmann, Dmitry Osipenko,
Zack Rusin, Matthew Brost, Thomas Hellstrom,
Oleksandr Andrushchenko, Helge Deller, Benjamin LaHaise,
Alexander Viro, Christian Brauner, Muchun Song, Oscar Salvador,
David Hildenbrand, Zi Yan, Baolin Wang, Liam R . Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Hugh Dickins, Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan,
Michal Hocko, Jann Horn, Pedro Falcato, Kees Cook,
Jaroslav Kysela, Takashi Iwai, linux-mips, linux-kernel,
linuxppc-dev, dri-devel, etnaviv, linux-arm-kernel,
linux-samsung-soc, intel-gfx, linux-arm-msm, freedreno, nouveau,
linux-rockchip, linux-tegra, virtualization, intel-xe, xen-devel,
linux-fbdev, linux-aio, linux-fsdevel, linux-mm, linux-sound
In-Reply-To: <3bb8bdc4788230c33102166d56cbc5abfad9d4cb.1782760670.git.ljs@kernel.org>
On Mon, 29 Jun 2026, Lorenzo Stoakes <ljs@kernel.org> wrote:
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 12 ++++++------
For i915,
Acked-by: Jani Nikula <jani.nikula@intel.com>
--
Jani Nikula, Intel
^ permalink raw reply
* Re: [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration
From: Thorsten Leemhuis @ 2026-06-30 12:06 UTC (permalink / raw)
To: Roman Ilin
Cc: Louis Chauvet, David Airlie, Simona Vetter, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard, Javier Martinez Canillas,
Dmitry Osipenko, dri-devel, virtualization, linux-kernel,
Peter Arnesen, Linux kernel regressions list
In-Reply-To: <20260613224434.96501-1-me@romanilin.is>
On 6/14/26 00:44, Roman Ilin wrote:
> When a CRTC's display mode carries a too small pixel clock,
> drm_calc_timestamping_constants() computes a frame duration that
> exceeds INT_MAX. drm_vblank_crtc.framedur_ns becomes negative.
> drm_crtc_vblank_start_timer() then arms the vblank hrtimer with this
> interval, after which vblank events are no longer delivered. Pending
> page flips never complete and the display appears frozen.
Roman, what's the status here? Does the problem still happen with
7.2-rc1? Sounds like it likely will, just want to make sure before
prodding Thomas about it, who authored and committed the change that
causes this regression.
Side note: CCing Peter Arnesen, who seems to be affected as well.
Quoting a message that was CCed to the regression list, but apparently
was rejected there:
"""
Hi everyone,
I am experiencing a similar Wayland display freeze on Kernel 7.0
described in Roman Ilin's recent patch thread ("drm/vblank: Don't arm
vblank timer with invalid frame duration").
Since my laptop consistently reproduces this freeze on boot, I am
reaching out to offer my hardware for testing if you need verification
that this patch resolves the issue on the newer Ryzen AI architectures.
My System:
* CPU / iGPU: AMD Ryzen AI 9 HX
* dGPU: NVIDIA GeForce RTX 5070
* OS: Fedora 44, Kubuntu 26.04 and Nobara Linux (Fedora-based) / Wayland
* Regression Status: Works perfectly on Kernel 6.19. Fails on Kernel 7.0.
* Tested Workarounds: Kernel parameters amdgpu.dcdebugmask=0x10, 0x410,
and amdgpu.sg_display=0 do not bypass the freeze.
Please let me know if you need me to pull specific dmesg logs or similar
from the frozen state, or if there is a specific patched branch you
would like me to compile and boot to verify the fix.
Regards
Peter
"""
Peter, would be great if you could test 7.2-rc1, too. And if you really
think the problem is the same, it might be worth trying Roman's patch.
Ciao, Thorsten
> This could be triggered on virtio-gpu guests that have dynamic resolution
> enabled: when the SPICE agent or the X server resizes the output, it
> submits a mode whose pixel clock is off by a factor of 1000, e.g.:
>
> clock = 406 kHz, htotal = 3152, vtotal = 2148
>
> framedur_ns = 3152 * 2148 * 1000000 / 406 = 16675852216 ns (~16.7 s)
>
> 16675852216 does not fit into an int and wraps to roughly -504000000.
> ns_to_ktime() then yields a negative interval and the timer stops working.
>
> Found by bisection, which pointed at commit a036f5fceedb ("drm/virtgpu:
> Use vblank timer"). That commit merely made virtio-gpu use the vblank
> timer and thereby exposed the pre-existing problem in the timer setup
> added by commit 74afeb812850 ("drm/vblank: Add vblank timer").
>
> Reject a non-positive frame duration in drm_crtc_vblank_start_timer() and
> return an error. enable_vblank then fails and the driver falls back to
> sending the vblank event immediately, as it did before the vblank timer
> was introduced. Valid modes are unaffected, and the timer self-heals on
> the next mode that has a sane clock.
>
> Fixes: 74afeb812850 ("drm/vblank: Add vblank timer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Roman Ilin <me@romanilin.is>
> ---
> Notes:
>
> Based on v7.1-rc7. Tested on 6.19 and 7.1-rc7.
>
> Open questions:
>
> This relies on the int overflow producing a negative value. The deeper
> issue is that drm_calc_timestamping_constants() truncates framedur_ns to
> int. Would you prefer to widen framedur_ns to s64, or to bound the
> interval here (e.g. reject framedur_ns above one second) so that any
> bogus interval is rejected regardless of sign?
>
> Should virtio-gpu additionally sanitize the user-supplied clock in its
> atomic_check (similar to vmwgfx for the clock==0 case) so the
> vblank-timer throttling is preserved for these resizes, instead of
> falling back to immediate events?
>
> drivers/gpu/drm/drm_vblank.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index f78bf37f1..557cd0bc8 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -2235,7 +2235,13 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
>
> drm_calc_timestamping_constants(crtc, &crtc->mode);
>
> + /*
> + * Return an error so the driver falls back to sending vblank events
> + * when a small mode clock yields a frame duration exceeding INT_MAX.
> + */
> + if (vblank->framedur_ns <= 0)
> + return -EINVAL;
> +
> spin_lock_irqsave(&vtimer->interval_lock, flags);
> vtimer->interval = ns_to_ktime(vblank->framedur_ns);
> spin_unlock_irqrestore(&vtimer->interval_lock, flags);
^ permalink raw reply
* Re: [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration
From: Thomas Zimmermann @ 2026-06-30 12:09 UTC (permalink / raw)
To: Thorsten Leemhuis, Roman Ilin
Cc: Louis Chauvet, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Javier Martinez Canillas, Dmitry Osipenko,
dri-devel, virtualization, linux-kernel, Peter Arnesen,
Linux kernel regressions list
In-Reply-To: <ef53342b-7c63-4681-8c5d-de86d9ee5541@leemhuis.info>
Hi,
please also see this bug report:
https://gitlab.freedesktop.org/spice/linux/vd_agent/-/work_items/52
Am 30.06.26 um 14:06 schrieb Thorsten Leemhuis:
> On 6/14/26 00:44, Roman Ilin wrote:
>> When a CRTC's display mode carries a too small pixel clock,
>> drm_calc_timestamping_constants() computes a frame duration that
>> exceeds INT_MAX. drm_vblank_crtc.framedur_ns becomes negative.
>> drm_crtc_vblank_start_timer() then arms the vblank hrtimer with this
>> interval, after which vblank events are no longer delivered. Pending
>> page flips never complete and the display appears frozen.
> Roman, what's the status here? Does the problem still happen with
> 7.2-rc1? Sounds like it likely will, just want to make sure before
> prodding Thomas about it, who authored and committed the change that
> causes this regression.
>
> Side note: CCing Peter Arnesen, who seems to be affected as well.
> Quoting a message that was CCed to the regression list, but apparently
> was rejected there:
>
> """
> Hi everyone,
>
> I am experiencing a similar Wayland display freeze on Kernel 7.0
> described in Roman Ilin's recent patch thread ("drm/vblank: Don't arm
> vblank timer with invalid frame duration").
>
> Since my laptop consistently reproduces this freeze on boot, I am
> reaching out to offer my hardware for testing if you need verification
> that this patch resolves the issue on the newer Ryzen AI architectures.
>
> My System:
> * CPU / iGPU: AMD Ryzen AI 9 HX
> * dGPU: NVIDIA GeForce RTX 5070
> * OS: Fedora 44, Kubuntu 26.04 and Nobara Linux (Fedora-based) / Wayland
> * Regression Status: Works perfectly on Kernel 6.19. Fails on Kernel 7.0.
> * Tested Workarounds: Kernel parameters amdgpu.dcdebugmask=0x10, 0x410,
> and amdgpu.sg_display=0 do not bypass the freeze.
>
> Please let me know if you need me to pull specific dmesg logs or similar
> from the frozen state, or if there is a specific patched branch you
> would like me to compile and boot to verify the fix.
>
> Regards
>
> Peter
> """
>
> Peter, would be great if you could test 7.2-rc1, too. And if you really
> think the problem is the same, it might be worth trying Roman's patch.
> Ciao, Thorsten
>
>> This could be triggered on virtio-gpu guests that have dynamic resolution
>> enabled: when the SPICE agent or the X server resizes the output, it
>> submits a mode whose pixel clock is off by a factor of 1000, e.g.:
>>
>> clock = 406 kHz, htotal = 3152, vtotal = 2148
>>
>> framedur_ns = 3152 * 2148 * 1000000 / 406 = 16675852216 ns (~16.7 s)
>>
>> 16675852216 does not fit into an int and wraps to roughly -504000000.
>> ns_to_ktime() then yields a negative interval and the timer stops working.
>>
>> Found by bisection, which pointed at commit a036f5fceedb ("drm/virtgpu:
>> Use vblank timer"). That commit merely made virtio-gpu use the vblank
>> timer and thereby exposed the pre-existing problem in the timer setup
>> added by commit 74afeb812850 ("drm/vblank: Add vblank timer").
>>
>> Reject a non-positive frame duration in drm_crtc_vblank_start_timer() and
>> return an error. enable_vblank then fails and the driver falls back to
>> sending the vblank event immediately, as it did before the vblank timer
>> was introduced. Valid modes are unaffected, and the timer self-heals on
>> the next mode that has a sane clock.
>>
>> Fixes: 74afeb812850 ("drm/vblank: Add vblank timer")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Roman Ilin <me@romanilin.is>
>> ---
>> Notes:
>>
>> Based on v7.1-rc7. Tested on 6.19 and 7.1-rc7.
>>
>> Open questions:
>>
>> This relies on the int overflow producing a negative value. The deeper
>> issue is that drm_calc_timestamping_constants() truncates framedur_ns to
>> int. Would you prefer to widen framedur_ns to s64, or to bound the
>> interval here (e.g. reject framedur_ns above one second) so that any
>> bogus interval is rejected regardless of sign?
>>
>> Should virtio-gpu additionally sanitize the user-supplied clock in its
>> atomic_check (similar to vmwgfx for the clock==0 case) so the
>> vblank-timer throttling is preserved for these resizes, instead of
>> falling back to immediate events?
>>
>> drivers/gpu/drm/drm_vblank.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>> index f78bf37f1..557cd0bc8 100644
>> --- a/drivers/gpu/drm/drm_vblank.c
>> +++ b/drivers/gpu/drm/drm_vblank.c
>> @@ -2235,7 +2235,13 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
>>
>> drm_calc_timestamping_constants(crtc, &crtc->mode);
>>
>> + /*
>> + * Return an error so the driver falls back to sending vblank events
>> + * when a small mode clock yields a frame duration exceeding INT_MAX.
>> + */
>> + if (vblank->framedur_ns <= 0)
>> + return -EINVAL;
>> +
>> spin_lock_irqsave(&vtimer->interval_lock, flags);
>> vtimer->interval = ns_to_ktime(vblank->framedur_ns);
>> spin_unlock_irqrestore(&vtimer->interval_lock, flags);
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v3 2/4] vhost/vsock: suppress EHOSTUNREACH fast-fail during CPR pause
From: Stefano Garzarella @ 2026-06-30 12:39 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <20260625155416.480669-3-andrey.drobyshev@virtuozzo.com>
On Thu, Jun 25, 2026 at 06:54:14PM +0300, Andrey Drobyshev wrote:
>Earlier commit bb26ed5f3a8b ("vhost/vsock: Refuse the connection
>immediately when guest isn't ready") added a fast-fail in
>vhost_transport_send_pkt(). It rejects every host send with -EHOSTUNREACH
>until the destination calls SET_RUNNING(1). The fast-fail condition checks
>whether device's backends are dropped, and if they're, the guest is
>considered to be not ready.
>
>However, there might be other reasons for backends to be nulled. In
>particular, when QEMU is performing CPR (checkpoint-restore) migration,
>device ownership is being RESET and SET again, which leads to backends
>drop and reattach. If we end up connecting during this window, an
>AF_VSOCK client gets -EHOSTUNREACH, which is wrong.
nit: IMO we should make it clear that this behavior has not yet been
implemented, so this patch is a preparation patch to support RESET. In
this way it is clear that it is not a fix to be backported.
>
>Add a 'started' flag which is set once in vhost_vsock_start() and is
>never cleared. The behaviour changes to:
>
> * When device was never started -> flag is unset -> no listener can
> exist yet -> fast-fail;
> * Once the device starts -> flag is set -> we don't fast-fail ->
> we queue and preserve during any later stop / CPR pause.
>
>Important caveat: after the first start, a connect during any stopped
>window is queued instead of fast-failed. That was the behaviour before
>the patch bb26ed5f3a8b, and we're restoring it now. However we still
>keep the behaviour originally intended by that commit (i.e. fast-fail if
>there's no real listener yet) while fixing the CPR path.
>
Suggested-by tag is nice to use in this case.
>Signed-off-by: Denis V. Lunev <den@openvz.org>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>---
> drivers/vhost/vsock.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index b12221ce6faf..bec6bcfd885f 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -61,6 +61,7 @@ struct vhost_vsock {
>
> u32 guest_cid;
> bool seqpacket_allow;
>+ bool started; /* set on first SET_RUNNING(1); never cleared */
`started` was my initial proposal when I was thiking we should have to
set it to false when the device is stopped.
Now I think this name is confusing, so what about `ever_started` to be
reused in the future, or just `fast_fail` since this is what that
variable is controlloing right now.
The rest LGTM.
Thanks,
Stefano
> };
>
> static u32 vhost_transport_get_local_cid(void)
>@@ -302,17 +303,12 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
> return -ENODEV;
> }
>
>- /* Fast-fail if the guest hasn't enabled the RX vq yet. Queuing the packet
>- * and making the caller wait is pointless: even if the guest manages to init
>- * within the timeout, it'll immediately reply with RST, because there's no
>- * listener on the port yet.
>- *
>- * vhost_vq_get_backend() without vq->mutex is acceptable here: locking
>- * the mutex would be too expensive in this hot path, and we already have
>- * all the outcomes covered: if the backend becomes NULL right after the check,
>- * vhost_transport_do_send_pkt() will check it under the mutex anyway.
>+ /* Fast-fail until the guest first enables the device (SET_RUNNING(1)).
>+ * Before that there is no listener, so queuing is pointless. 'started'
>+ * is never cleared, so once we're up we keep queuing across later
>+ * stop / CPR-pause windows.
> */
>- if (unlikely(!data_race(vhost_vq_get_backend(&vsock->vqs[VSOCK_VQ_RX])))) {
>+ if (unlikely(!READ_ONCE(vsock->started))) {
> rcu_read_unlock();
> kfree_skb(skb);
> return -EHOSTUNREACH;
>@@ -640,6 +636,11 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
> mutex_unlock(&vq->mutex);
> }
>
>+ /* Set 'started' flag on the first start; never cleared, so send_pkt
>+ * keeps queuing (instead of fast-failing) on later stop / CPR pauses.
>+ */
>+ WRITE_ONCE(vsock->started, true);
>+
> /* Some packets may have been queued before the device was started,
> * let's kick the send worker to send them.
> */
>@@ -728,6 +729,7 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
>
> vsock->guest_cid = 0; /* no CID assigned yet */
> vsock->seqpacket_allow = false;
>+ vsock->started = false;
>
> atomic_set(&vsock->queued_replies, 0);
>
>--
>2.47.1
>
^ permalink raw reply
* Re: [PATCH v3 3/4] vhost/vsock: re-scan TX virtqueue on device start
From: Stefano Garzarella @ 2026-06-30 12:45 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <20260625155416.480669-4-andrey.drobyshev@virtuozzo.com>
On Thu, Jun 25, 2026 at 06:54:15PM +0300, Andrey Drobyshev wrote:
>During QEMU CPR live-update (and VHOST_RESET_OWNER in general) the guest
>keeps running while the host drops and later re-attaches vhost backends.
>If the guest adds a buffer to the TX virtqueue (guest->host) and kicks
>while the backend is temporarily NULL (between vhost_vsock_drop_backends()
>and the next vhost_vsock_start()), then the kick is delivered to the
>vhost worker, handle_tx_kick() sees a NULL backend and returns, and the
>kick signal is consumed. The buffer is then left in the ring.
>
>Then upon device start vhost_vsock_start() only re-kicks the RX send
>worker, never the TX VQ, so the buffer is processed only if the guest
>happens to kick again. But if the guest itself is now waiting for data
>from the host, it will never kick TX VQ again, and we end up in a
>deadlock.
>
>The issue itself is pre-existing, but it only manifests during a brief
Why "brief"? I mean, there's no limit, and the user process could stay
there forever, right?
>pause caused by VHOST_RESET_OWNER. Namely, the deadlock is reproduced
Again, please make it clear that VHOST_RESET_OWNER support will come
later, so this is in prepartion for it.
>during active host->guest socat data transfer under multiple consecutive
>CPR live-update's.
>
>To fix this, in vhost_vsock_start(), after kicking the RX send worker, also
>queue the TX vq poll so any buffers the guest enqueued while we were paused
>get scanned.
>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>---
> drivers/vhost/vsock.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index bec6bcfd885f..81d4f7209719 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -646,6 +646,13 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
> */
> vhost_vq_work_queue(&vsock->vqs[VSOCK_VQ_RX], &vsock->send_pkt_work);
>
>+ /*
>+ * Some packets might've also been queued in TX VQ. That is the case
>+ * during the brief device pause caused by VHOST_RESET_OWNER. Re-scan
Ditto about "brief", I don't think is adding anything.
BTW the code LGTM.
Thanks,
Stefano
>+ * the TX VQ here, mirroring the RX send-worker kick above.
>+ */
>+ vhost_poll_queue(&vsock->vqs[VSOCK_VQ_TX].poll);
>+
> mutex_unlock(&vsock->dev.mutex);
> return 0;
>
>--
>2.47.1
>
^ permalink raw reply
* Re: [PATCH] drm/vblank: Don't arm vblank timer with invalid frame duration
From: Thomas Zimmermann @ 2026-06-30 12:58 UTC (permalink / raw)
To: Roman Ilin, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter, Ville Syrjälä
Cc: Louis Chauvet, Javier Martinez Canillas, Dmitry Osipenko,
dri-devel, virtualization, linux-kernel
In-Reply-To: <20260613224434.96501-1-me@romanilin.is>
(cc Ville)
Hi,
thanks for addressing the issue.
Am 14.06.26 um 00:44 schrieb Roman Ilin:
> When a CRTC's display mode carries a too small pixel clock,
> drm_calc_timestamping_constants() computes a frame duration that
> exceeds INT_MAX. drm_vblank_crtc.framedur_ns becomes negative.
> drm_crtc_vblank_start_timer() then arms the vblank hrtimer with this
> interval, after which vblank events are no longer delivered. Pending
> page flips never complete and the display appears frozen.
>
> This could be triggered on virtio-gpu guests that have dynamic resolution
> enabled: when the SPICE agent or the X server resizes the output, it
> submits a mode whose pixel clock is off by a factor of 1000, e.g.:
'off by' as in it should be in Hz rather than kHz.
>
> clock = 406 kHz, htotal = 3152, vtotal = 2148
>
> framedur_ns = 3152 * 2148 * 1000000 / 406 = 16675852216 ns (~16.7 s)
>
> 16675852216 does not fit into an int and wraps to roughly -504000000.
> ns_to_ktime() then yields a negative interval and the timer stops working.
>
> Found by bisection, which pointed at commit a036f5fceedb ("drm/virtgpu:
> Use vblank timer"). That commit merely made virtio-gpu use the vblank
> timer and thereby exposed the pre-existing problem in the timer setup
> added by commit 74afeb812850 ("drm/vblank: Add vblank timer").
>
> Reject a non-positive frame duration in drm_crtc_vblank_start_timer() and
> return an error. enable_vblank then fails and the driver falls back to
> sending the vblank event immediately, as it did before the vblank timer
> was introduced. Valid modes are unaffected, and the timer self-heals on
> the next mode that has a sane clock.
>
> Fixes: 74afeb812850 ("drm/vblank: Add vblank timer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Roman Ilin <me@romanilin.is>
> ---
> Notes:
>
> Based on v7.1-rc7. Tested on 6.19 and 7.1-rc7.
>
> Open questions:
>
> This relies on the int overflow producing a negative value. The deeper
> issue is that drm_calc_timestamping_constants() truncates framedur_ns to
> int. Would you prefer to widen framedur_ns to s64, or to bound the
> interval here (e.g. reject framedur_ns above one second) so that any
> bogus interval is rejected regardless of sign?
Generally speaking, I think we should not accept such a bugos mode in
the first place. But this would require changes to the mode-setting code
that are too invasive for a bug fix.
So, if anything, we should try to detect the problem in
drm_calc_timestamping_constants(). Let's make the helper return errno
codes instead of failing silently. Within the helper, let's do the
following changes:
- declare frame_size an unsigned int
- declare linedur_ns and framedur_ns of type u64
This should avoid possible overflows in the code. And before assigning
linedur_ns and framedur_ns to the vblank fields, test them against INT_MAX.
Maybe at [1]. Using drm_WARN_ON_ONCE is likely a good idea for future
debugging.
if (drm_WARN_ON_ONCE(dev, linedur_ns > INT_MAX) || drm_WARN_ON_ONCE(dev,
framedur_ns > INT_MAX))
return -EINVAL
[1]
https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/drm_vblank.c#L662
>
> Should virtio-gpu additionally sanitize the user-supplied clock in its
> atomic_check (similar to vmwgfx for the clock==0 case) so the
> vblank-timer throttling is preserved for these resizes, instead of
> falling back to immediate events?
>
> drivers/gpu/drm/drm_vblank.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index f78bf37f1..557cd0bc8 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -2235,7 +2235,13 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
>
> drm_calc_timestamping_constants(crtc, &crtc->mode);
>
> + /*
> + * Return an error so the driver falls back to sending vblank events
> + * when a small mode clock yields a frame duration exceeding INT_MAX.
> + */
> + if (vblank->framedur_ns <= 0)
> + return -EINVAL;
Here, you would just forward the error upwards in the call stack.
Best regards
Thomas
> +
> spin_lock_irqsave(&vtimer->interval_lock, flags);
> vtimer->interval = ns_to_ktime(vblank->framedur_ns);
> spin_unlock_irqrestore(&vtimer->interval_lock, flags);
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH] drm/virtio: bound EDID block reads to the response buffer
From: Dmitry Osipenko @ 2026-06-30 13:17 UTC (permalink / raw)
To: hexlabsecurity, David Airlie, Gerd Hoffmann
Cc: linux-kernel, Gurchetan Singh, Chia-I Wu, dri-devel,
virtualization
In-Reply-To: <20260620-b4-disp-22bba7bf-v1-1-b95924cee742@proton.me>
On 6/21/26 05:43, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> virtio_get_edid_block() validates the read offset only against the
> device-supplied resp->size field, never against the fixed-size resp->edid
> array. The EDID block index is driven by the device-supplied extension
> count, so a malicious virtio-gpu backend can advertise a large size
> together with a high block count and read far past the array into adjacent
> kernel memory, which is then surfaced in the parsed EDID (an out-of-bounds
> read / info leak).
>
> Also reject any read whose end exceeds the size of the edid array.
> Conforming EDID responses stay within the array and are unaffected.
>
> Fixes: b4b01b4995fb ("drm/virtio: add edid support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
> drivers/gpu/drm/virtio/virtgpu_vq.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 67865810a2e7..c8b9475a7472 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -897,7 +897,8 @@ static int virtio_get_edid_block(void *data, u8 *buf,
> struct virtio_gpu_resp_edid *resp = data;
> size_t start = block * EDID_LENGTH;
>
> - if (start + len > le32_to_cpu(resp->size))
> + if (start + len > le32_to_cpu(resp->size) ||
> + start + len > sizeof(resp->edid))
> return -EINVAL;
> memcpy(buf, resp->edid + start, len);
> return 0;
Applied to misc-fixes, thanks!
--
Best regards,
Dmitry
^ permalink raw reply
* Re: [PATCH] drm/virtio: fail init on display-info timeout
From: Dmitry Osipenko @ 2026-06-30 13:36 UTC (permalink / raw)
To: Pengpeng Hou, David Airlie, Gerd Hoffmann
Cc: Gurchetan Singh, Chia-I Wu, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Simona Vetter, dri-devel, virtualization,
linux-kernel
In-Reply-To: <20260625030202.82590-1-pengpeng@iscas.ac.cn>
On 6/25/26 06:02, Pengpeng Hou wrote:
> virtio_gpu_init() sends GET_DISPLAY_INFO when scanouts are present and
> waits for display_info_pending to clear. If the response never arrives,
> the wait result is ignored and probe still succeeds.
>
> Return -ETIMEDOUT on display-info timeout. Because this happens after
> virtio_device_ready(), reset the device and tear down modesetting before
> using the existing vbuf and virtqueue cleanup path.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> drivers/gpu/drm/virtio/virtgpu_kms.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index cfde9f573df6..31209bea97ae 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -262,11 +262,19 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
> virtio_gpu_cmd_get_edids(vgdev);
> virtio_gpu_cmd_get_display_info(vgdev);
> virtio_gpu_notify(vgdev);
> - wait_event_timeout(vgdev->resp_wq, !vgdev->display_info_pending,
> - 5 * HZ);
> + if (!wait_event_timeout(vgdev->resp_wq,
> + !vgdev->display_info_pending,
> + 5 * HZ)) {
> + DRM_ERROR("timed out waiting for display info\n");
> + ret = -ETIMEDOUT;
> + goto err_ready;
> + }
> }
> return 0;
>
> +err_ready:
> + virtio_reset_device(vgdev->vdev);
> + virtio_gpu_modeset_fini(vgdev);
> err_scanouts:
> virtio_gpu_free_vbufs(vgdev);
> err_vbufs:
Rebased and applied to misc-next, thanks!
--
Best regards,
Dmitry
^ permalink raw reply
* Re: [PATCH v3 4/4] vhost/vsock: add VHOST_RESET_OWNER ioctl
From: Stefano Garzarella @ 2026-06-30 13:40 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <20260625155416.480669-5-andrey.drobyshev@virtuozzo.com>
On Thu, Jun 25, 2026 at 06:54:16PM +0300, Andrey Drobyshev wrote:
>From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>
>This ioctl is needed for QEMU's CPR (checkpoint-restore) migration of
>the guest with vhost-vsock device. For this to work, we need to reset
>the device ownership on the source side by calling RESET_OWNER, and then
>claim it on the dest side by calling SET_OWNER. We expect not to lose any
>AF_VSOCK connection while this happens.
>
>RESET_OWNER keeps the guest CID hashed, so that connections survive. That
>leaves the device reachable by a lockless send/cancel path while the worker
>is being torn down: a concurrent vhost_transport_send_pkt() or
>vhost_transport_cancel_pkt() can call vhost_vq_work_queue() as
>vhost_workers_free() frees the worker. That might cause a use-after-free
>of vq->worker. In addition, any work queued onto the dying worker leaves
>VHOST_WORK_QUEUED stuck, stalling send_pkt_queue after resume.
>
>Fence the send/cancel paths around the teardown: send_pkt()/cancel_pkt()
>only kick the worker while the backend is alive. And reset_owner() calls
>synchronize_rcu() after drop_backends() so in-flight send/cancel finish
>before the worker is freed.
>
>Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>---
> drivers/vhost/vsock.c | 51 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 49 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index 81d4f7209719..f0a0aa7d3200 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -318,7 +318,14 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
> atomic_inc(&vsock->queued_replies);
>
> virtio_vsock_skb_queue_tail(&vsock->send_pkt_queue, skb);
>- vhost_vq_work_queue(&vsock->vqs[VSOCK_VQ_RX], &vsock->send_pkt_work);
>+
>+ /* Skip the kick once the backend is gone (stop/RESET_OWNER); the skb
>+ * stays queued and vhost_vsock_start() drains it. Pairs with the
>+ * synchronize_rcu() in vhost_vsock_reset_owner().
>+ */
Please explain better (as done by commit bb26ed5f3a8b ("vhost/vsock:
Refuse the connection immediately when guest isn't ready") in the
comment removed by this seris) why we can use vhost_vq_get_backend()
without vq->mutex held.
>+ if (data_race(vhost_vq_get_backend(&vsock->vqs[VSOCK_VQ_RX])))
>+ vhost_vq_work_queue(&vsock->vqs[VSOCK_VQ_RX],
>+ &vsock->send_pkt_work);
BTW I'm now confused about what we are preventing here. A better
explanation should be added both in the commit and in the comment,
because it's hard to understand what we're preventing.
That said, if there is a problem, perhaps it should be fixed in vhost.c,
because it seems more like a generic issue.
vhost_vq_work_queue() has `worker = rcu_dereference(vq->worker);` so
should already prevent UAF, no?
Or maybe vhost_workers_free() is missing a synchronize_rcu()?
>
> rcu_read_unlock();
> return len;
>@@ -346,7 +353,15 @@ vhost_transport_cancel_pkt(struct vsock_sock *vsk)
> int new_cnt;
>
> new_cnt = atomic_sub_return(cnt, &vsock->queued_replies);
>- if (new_cnt + cnt >= tx_vq->num && new_cnt < tx_vq->num)
>+
>+ /* Skip the kick once the backend is gone (stop/RESET_OWNER):
>+ * vhost_poll_queue() would touch the worker which is being freed
>+ * by teardown, e.g. on RESET_OWNER. Pairs with the
>+ * synchronize_rcu() in vhost_vsock_reset_owner(). The TX VQ is
Ditto about the comment.
>+ * re-kicked by vhost_vsock_start().
>+ */
>+ if (data_race(vhost_vq_get_backend(tx_vq)) &&
>+ new_cnt + cnt >= tx_vq->num && new_cnt < tx_vq->num)
> vhost_poll_queue(&tx_vq->poll);
> }
>
>@@ -903,6 +918,36 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
> return -EFAULT;
> }
>
>+static int vhost_vsock_reset_owner(struct vhost_vsock *vsock)
Why returning int?
We are defining err as long here, also the caller vhost_vsock_dev_ioctl()
returns long, so it is not clear to me why here we are not just
returning long.
>+{
>+ struct vhost_iotlb *umem;
>+ long err;
>+
>+ mutex_lock(&vsock->dev.mutex);
>+ err = vhost_dev_check_owner(&vsock->dev);
>+ if (err)
>+ goto done;
>+ umem = vhost_dev_reset_owner_prepare();
>+ if (!umem) {
>+ err = -ENOMEM;
>+ goto done;
>+ }
>+ vhost_vsock_drop_backends(vsock);
>+
>+ /* Let in-flight send_pkt() callers stop touching the worker before the
>+ * flush + free below. Pairs with the backend check in
>+ * vhost_transport_send_pkt().
This is also paired with vhost_transport_cancel_pkt(), so please update
this comment.
>+ */
>+ synchronize_rcu();
>+
>+ vhost_vsock_flush(vsock);
>+ vhost_dev_stop(&vsock->dev);
>+ vhost_dev_reset_owner(&vsock->dev, umem);
>+done:
>+ mutex_unlock(&vsock->dev.mutex);
>+ return err;
>+}
>+
> static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
> unsigned long arg)
> {
>@@ -946,6 +991,8 @@ static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
> return -EOPNOTSUPP;
> vhost_set_backend_features(&vsock->dev, features);
> return 0;
>+ case VHOST_RESET_OWNER:
>+ return vhost_vsock_reset_owner(vsock);
> default:
> mutex_lock(&vsock->dev.mutex);
> r = vhost_dev_ioctl(&vsock->dev, ioctl, argp);
>--
>2.47.1
>
^ permalink raw reply
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock
From: Dmitry Osipenko @ 2026-06-30 13:46 UTC (permalink / raw)
To: Ryosuke Yasuoka, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas
Cc: dri-devel, virtualization, linux-kernel
In-Reply-To: <20260630-virtiogpu_syzbot-v1-1-0aa06630750e@redhat.com>
Hi,
On 6/30/26 12:16, Ryosuke Yasuoka wrote:
> A probe-time deadlock can occur between the dequeue worker and
> drm_client_register(). During probe, drm_client_register() holds
> clientlist_mutex and calls the fbdev hotplug callback, which triggers an
> atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs()
> waiting for virtqueue space. The dequeue worker that would free that
> space calls virtio_gpu_cmd_get_display_info_cb(), which invokes
> drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting
> to acquire the same clientlist_mutex. Since wake_up() is only called
> after the resp_cb loop, the probe thread is never woken and both threads
> deadlock.
>
> Fix this by deferring the hotplug notification from
> virtio_gpu_cmd_get_display_info_cb() to a separate work item. The
> display data (outputs[i].info) is still updated synchronously in the
> callback, and the deferred work only triggers a re-probe notification to
> DRM clients.
>
> Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client")
> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> ---
> drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +++
> drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++
> drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++++++++--
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 7449907754a4..27ffa4697ae9 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -264,6 +264,8 @@ struct virtio_gpu_device {
>
> struct work_struct config_changed_work;
>
> + struct work_struct hotplug_work;
> +
> struct work_struct obj_free_work;
> spinlock_t obj_free_lock;
> struct list_head obj_free_list;
> @@ -350,6 +352,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
> uint32_t x, uint32_t y,
> struct virtio_gpu_object_array *objs,
> struct virtio_gpu_fence *fence);
> +void virtio_gpu_hotplug_work_func(struct work_struct *work);
> void virtio_gpu_panic_cmd_resource_flush(struct virtio_gpu_device *vgdev,
> uint32_t resource_id,
> uint32_t x, uint32_t y,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index cfde9f573df6..cfb532ba43a4 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -154,6 +154,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
> INIT_WORK(&vgdev->config_changed_work,
> virtio_gpu_config_changed_work_func);
>
> + INIT_WORK(&vgdev->hotplug_work, virtio_gpu_hotplug_work_func);
> +
> INIT_WORK(&vgdev->obj_free_work,
> virtio_gpu_array_put_free_work);
> INIT_LIST_HEAD(&vgdev->obj_free_list);
> @@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
> flush_work(&vgdev->obj_free_work);
> flush_work(&vgdev->ctrlq.dequeue_work);
> flush_work(&vgdev->cursorq.dequeue_work);
> + flush_work(&vgdev->hotplug_work);
> flush_work(&vgdev->config_changed_work);
> virtio_reset_device(vgdev->vdev);
> vgdev->vdev->config->del_vqs(vgdev->vdev);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 67865810a2e7..084d98f5dc7b 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -816,6 +816,15 @@ virtio_gpu_cmd_resource_detach_backing(struct virtio_gpu_device *vgdev,
> virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence);
> }
>
> +void virtio_gpu_hotplug_work_func(struct work_struct *work)
> +{
> + struct virtio_gpu_device *vgdev =
> + container_of(work, struct virtio_gpu_device, hotplug_work);
> +
> + if (!drm_helper_hpd_irq_event(vgdev->ddev))
> + drm_kms_helper_hotplug_event(vgdev->ddev);
> +}
> +
> static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_vbuffer *vbuf)
> {
> @@ -841,8 +850,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
> spin_unlock(&vgdev->display_info_lock);
> wake_up(&vgdev->resp_wq);
>
> - if (!drm_helper_hpd_irq_event(vgdev->ddev))
> - drm_kms_helper_hotplug_event(vgdev->ddev);
> + schedule_work(&vgdev->hotplug_work);
> }
>
> static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,
Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), placing it after wait_event_timeout(display_info_pending)? This will avoid additional work_struct that otherwise needs to be cancelled in virtio_gpu_init() on the timeout.
--
Best regards,
Dmitry
^ permalink raw reply
* Re: [PATCH] drm/virtio: fail init on display-info timeout
From: Dmitry Osipenko @ 2026-06-30 14:42 UTC (permalink / raw)
To: Pengpeng Hou, David Airlie, Gerd Hoffmann
Cc: Gurchetan Singh, Chia-I Wu, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Simona Vetter, dri-devel, virtualization,
linux-kernel
In-Reply-To: <f5c5fc6c-3c6e-4068-80d7-f1acebff7168@collabora.com>
On 6/30/26 16:36, Dmitry Osipenko wrote:
> On 6/25/26 06:02, Pengpeng Hou wrote:
>> virtio_gpu_init() sends GET_DISPLAY_INFO when scanouts are present and
>> waits for display_info_pending to clear. If the response never arrives,
>> the wait result is ignored and probe still succeeds.
>>
>> Return -ETIMEDOUT on display-info timeout. Because this happens after
>> virtio_device_ready(), reset the device and tear down modesetting before
>> using the existing vbuf and virtqueue cleanup path.
>>
>> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
>> ---
>> drivers/gpu/drm/virtio/virtgpu_kms.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
>> index cfde9f573df6..31209bea97ae 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
>> @@ -262,11 +262,19 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
>> virtio_gpu_cmd_get_edids(vgdev);
>> virtio_gpu_cmd_get_display_info(vgdev);
>> virtio_gpu_notify(vgdev);
>> - wait_event_timeout(vgdev->resp_wq, !vgdev->display_info_pending,
>> - 5 * HZ);
>> + if (!wait_event_timeout(vgdev->resp_wq,
>> + !vgdev->display_info_pending,
>> + 5 * HZ)) {
>> + DRM_ERROR("timed out waiting for display info\n");
>> + ret = -ETIMEDOUT;
>> + goto err_ready;
>> + }
>> }
>> return 0;
>>
>> +err_ready:
>> + virtio_reset_device(vgdev->vdev);
>> + virtio_gpu_modeset_fini(vgdev);
>> err_scanouts:
>> virtio_gpu_free_vbufs(vgdev);
>> err_vbufs:
>
> Rebased and applied to misc-next, thanks!
Now see [1] having valid point on a leaked memory allocations. It's not
trivial to free that memory properly. Perhaps best will be to revert
this change and use wait_event() without timeout, will think further on
it before proceeding.
[1]
https://sashiko.dev/#/patchset/20260625030202.82590-1-pengpeng%40iscas.ac.cn
--
Best regards,
Dmitry
^ permalink raw reply
* Re: [PATCH 01/13] mm: introduce vma_flags_can_grow() and vma_can_grow()
From: Zi Yan @ 2026-06-30 15:09 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Thomas Bogendoerfer, Madhavan Srinivasan,
Michael Ellerman, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Lucas Stach,
Inki Dae, Seung-Woo Kim, Kyungmin Park, Krzysztof Kozlowski,
Peter Griffin, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Rob Clark, Dmitry Baryshkov, Lyude Paul,
Danilo Krummrich, Tomi Valkeinen, Sandy Huang, Heiko Stübner,
Andy Yan, Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Gerd Hoffmann, Dmitry Osipenko, Zack Rusin, Matthew Brost,
Thomas Hellstrom, Oleksandr Andrushchenko, Helge Deller,
Benjamin LaHaise, Alexander Viro, Christian Brauner, Muchun Song,
Oscar Salvador, David Hildenbrand, Baolin Wang, Liam R . Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Hugh Dickins, Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan,
Michal Hocko, Jann Horn, Pedro Falcato, Kees Cook,
Jaroslav Kysela, Takashi Iwai, linux-mips, linux-kernel,
linuxppc-dev, dri-devel, etnaviv, linux-arm-kernel,
linux-samsung-soc, intel-gfx, linux-arm-msm, freedreno, nouveau,
linux-rockchip, linux-tegra, virtualization, intel-xe, xen-devel,
linux-fbdev, linux-aio, linux-fsdevel, linux-mm, linux-sound
In-Reply-To: <akNucoP3eaDN2_Vz@lucifer>
On Tue Jun 30, 2026 at 3:38 AM EDT, Lorenzo Stoakes wrote:
> On Mon, Jun 29, 2026 at 04:26:18PM -0400, Zi Yan wrote:
>> On Mon Jun 29, 2026 at 3:25 PM EDT, Lorenzo Stoakes wrote:
>> > These test whether the VMA has stack sematics, i.e. is able to grow upwards
>> > or downwards depending on the architecture.
>> >
>> > In order to account for arches which do not support upward-growing stacks,
>> > introduce VMA_GROWSUP whose definition depends on the architecture
>> > supporting it, and use vma_flags_test_single_mask() in vma_flags_can_grow()
>> > to account for this.
>> >
>> > Update the VMA userland tests to reflect the changes
>> >
>> > No functional change intended.
>> >
>> > Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>> > ---
>> > include/linux/mm.h | 21 ++++++++++++++++++---
>> > tools/testing/vma/include/dup.h | 4 ++++
>> > 2 files changed, 22 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/include/linux/mm.h b/include/linux/mm.h
>> > index 868b2334bff3..cf7df1569052 100644
>> > --- a/include/linux/mm.h
>> > +++ b/include/linux/mm.h
>> > @@ -472,6 +472,7 @@ enum {
>> > #define VM_SAO INIT_VM_FLAG(SAO)
>> > #elif defined(CONFIG_PARISC)
>> > #define VM_GROWSUP INIT_VM_FLAG(GROWSUP)
>> > +#define VMA_GROWSUP mk_vma_flags(VMA_GROWSUP_BIT)
>> > #elif defined(CONFIG_SPARC64)
>> > #define VM_SPARC_ADI INIT_VM_FLAG(SPARC_ADI)
>> > #define VM_ARCH_CLEAR INIT_VM_FLAG(ARCH_CLEAR)
>> > @@ -483,6 +484,7 @@ enum {
>> > #endif
>> > #ifndef VM_GROWSUP
>> > #define VM_GROWSUP VM_NONE
>> > +#define VMA_GROWSUP EMPTY_VMA_FLAGS
>> > #endif
>> > #ifdef CONFIG_ARM64_MTE
>> > #define VM_MTE INIT_VM_FLAG(MTE)
>> > @@ -1563,11 +1565,24 @@ static inline bool vma_is_initial_stack(const struct vm_area_struct *vma)
>> > vma->vm_end >= vma->vm_mm->start_stack;
>> > }
>> >
>> > -static inline bool vma_is_temporary_stack(const struct vm_area_struct *vma)
>> > +static inline bool vma_flags_can_grow(const vma_flags_t *flags)
>> > {
>> > - int maybe_stack = vma->vm_flags & (VM_GROWSDOWN | VM_GROWSUP);
>> > + if (vma_flags_test_single_mask(flags, VMA_GROWSUP))
>> > + return true;
>> > + if (vma_flags_test(flags, VMA_GROWSDOWN_BIT))
>> > + return true;
>> > +
>> > + return false;
>> > +}
>> >
>> > - if (!maybe_stack)
>> > +static inline bool vma_can_grow(const struct vm_area_struct *vma)
>> > +{
>> > + return vma_flags_can_grow(&vma->flags);
>>
>> Would it save vma_flags_can_grow() if we do below?
>>
>> return vma_test(vma, VMA_GROWSDOWN_BIT) || vma_test_single_mask(vma, VMA_GROWSUP);
>>
>> I find these two functions when I am reading mm.h.
>
> Yeah but we require vma_flags_can_grow() for code in mmap.c, the majority of
> checks of this have only vma_flags_t to work with not a VMA :)
>
Got it. I think I need to finish this series. :)
>>
>> > +}
>> > +
>> > +static inline bool vma_is_temporary_stack(const struct vm_area_struct *vma)
>> > +{
>> > + if (!vma_can_grow(vma))
>> > return false;
>> >
>> > if ((vma->vm_flags & VM_STACK_INCOMPLETE_SETUP) ==
>> > diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
>> > index 5d7d0afd7765..6f5bcd7fbcd8 100644
>> > --- a/tools/testing/vma/include/dup.h
>> > +++ b/tools/testing/vma/include/dup.h
>> > @@ -245,8 +245,10 @@ enum {
>> > #define VM_STACK INIT_VM_FLAG(STACK)
>> > #ifdef CONFIG_STACK_GROWS_UP
>> > #define VM_STACK_EARLY INIT_VM_FLAG(STACK_EARLY)
>> > +#define VMA_STACK_EARLY mk_vma_flags(VMA_STACK_EARLY_BIT)
>> > #else
>> > #define VM_STACK_EARLY VM_NONE
>> > +#define VMA_STACK_EARLY EMPTY_VMA_FLAGS
>> > #endif
>> > #ifdef CONFIG_ARCH_HAS_PKEYS
>> > #define VM_PKEY_SHIFT ((__force int)VMA_HIGH_ARCH_0_BIT)
>> > @@ -315,6 +317,8 @@ enum {
>> >
>> > /* Bits set in the VMA until the stack is in its final location */
>> > #define VM_STACK_INCOMPLETE_SETUP (VM_RAND_READ | VM_SEQ_READ | VM_STACK_EARLY)
>> > +#define VMA_STACK_INCOMPLETE_SETUP append_vma_flags( \
>> > + VMA_STACK_EARLY, VMA_RAND_READ_BIT, VMA_SEQ_READ_BIT)
>> >
>> > #define TASK_EXEC_BIT ((current->personality & READ_IMPLIES_EXEC) ? \
>> > VM_EXEC_BIT : VM_READ_BIT)
>>
>> Why are VMA_STACK_EARLY and VMA_STACK_INCOMPLETE_SETUP added here but
>> not in mm.h?
>
> Yeah urgh oops my bad. It doesn't really break anything but I'll fix it if a
> respin is needed...
Sure.
--
Best Regards,
Yan, Zi
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox