* Re: [PATCH v2] virtio-pmem: allocate flush bio from a driver-private bio_set
From: Christoph Hellwig @ 2026-07-10 5:45 UTC (permalink / raw)
To: Joseph Qi
Cc: Pankaj Gupta, Christoph Hellwig, Baokun Li, virtualization,
linux-kernel
In-Reply-To: <20260709124455.1547912-1-joseph.qi@linux.alibaba.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [RFC] virtio_balloon: fix Use-After-Free in page reporting during PM freeze
From: Link Lin @ 2026-07-10 0:45 UTC (permalink / raw)
To: Andrew Morton
Cc: Vlastimil Babka, Michael S . Tsirkin, David Hildenbrand,
virtualization, linux-mm, linux-kernel, prasin, rientjes, duenwen,
jasowang, xuanzhuo, Ammar Faizi, jiaqiyan, ahwilkins, Greg Thelen,
Alexander Duyck, stable
In-Reply-To: <20260709161253.6b5e9ba349f70a3ebfb8180f@linux-foundation.org>
On Thu, 9 Jul 2026 at 16:12, Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Thu, 9 Jul 2026 22:43:30 +0000 Link Lin <linkl@google.com> wrote:
>
> > Fix this by:
> > 1. Unregistering page reporting in virtballoon_freeze() prior to calling
> > remove_common(). This clears the RCU pr_dev_info pointer and flushes/
> > cancels prdev->work on system_wq via cancel_delayed_work_sync().
> > 2. Re-registering page reporting in virtballoon_restore() after the
> > virtqueues are re-initialized and virtio_device_ready() has been called.
> > 3. Unwinding virtqueue initialization via remove_common() in
> > virtballoon_restore() if page_reporting_register() fails.
>
> AI review thinks the patch didn't do the above:
> https://sashiko.dev/#/patchset/20260709224330.946683-1-linkl@google.com
The AI reviewer might not have parsed the entirety of the fix I proposed.
The patch submitted definitely includes the changes to virtballoon_restore()
for steps 2 and 3 (re-registering page reporting and unwinding init_vqs
on failure). It seems the AI failed to parse the diff correctly. See:
+ if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) {
+ ret = page_reporting_register(&vb->pr_dev_info);
+ if (ret)
+ goto out_remove_vqs;
+ }
+
if (towards_target(vb))
virtballoon_changed(vdev);
update_balloon_size(vb);
return 0;
+
+out_remove_vqs:
+ remove_common(vb);
+ return ret;
}
> It also might have found a couple of pre-existing bugs in there.
Indeed. Regarding the first pre-existing bug found by the AI (leaving the
OOM notifier registered during suspend, leading to a UAF if memory pressure
spikes during S4 hibernation):
I actually addressed this in the commit message:
"(Note: The OOM Notifier and Shrinker/Free Page Hinting features suffer
from an identical lifecycle flaw and are also vulnerable to UAFs during
S4 hibernation when memory pressure spikes. This patch focuses on Free
Page Reporting, which runs periodically, to ensure clean backports to
stable kernels)."
Regarding the second pre-existing bug the AI flagged (leaving uncancelled
works on system_freezable_wq if virtballoon_restore fails on the cold path):
the AI is correct that this asynchronous work cancellation failure exists.
Since these are separate, pre-existing lifecycle bugs, would you prefer I
roll fixes for the OOM notifier, shrinker/free page hinting, and work
cancellations into a v2 of this patch, or submit them as a separate patch
series to keep the stable backports clean?
Thanks,
Link
^ permalink raw reply
* Re: [RFC] virtio_balloon: fix Use-After-Free in page reporting during PM freeze
From: Andrew Morton @ 2026-07-09 23:12 UTC (permalink / raw)
To: Link Lin
Cc: Vlastimil Babka, Michael S . Tsirkin, David Hildenbrand,
virtualization, linux-mm, linux-kernel, prasin, rientjes, duenwen,
jasowang, xuanzhuo, Ammar Faizi, jiaqiyan, ahwilkins, Greg Thelen,
Alexander Duyck, stable
In-Reply-To: <20260709224330.946683-1-linkl@google.com>
On Thu, 9 Jul 2026 22:43:30 +0000 Link Lin <linkl@google.com> wrote:
> During system power management freeze (e.g. ACPI S3 suspend or S4
> hibernation), virtballoon_freeze() calls remove_common() to reset the
> virtio device and delete all virtqueues via vdev->config->del_vqs().
> However, unlike virtballoon_remove(), virtballoon_freeze() fails to call
> page_reporting_unregister(&vb->pr_dev_info).
>
> ...
>
> If memory is freed into the buddy allocator or a delayed work timer
> expires while the device is being frozen, page_reporting_process() fires
> on system_wq and calls virtballoon_free_page_report(). This function
> passes vb->reporting_vq into virtqueue_add_inbuf() / virtqueue_add_split().
> Because the virtqueues were already destroyed by del_vqs(), this results
> in a Use-After-Free / General Protection Fault:
>
> [ 250.709271] general protection fault, probably for non-canonical address 0x7f728084daf08d5e: 0000 [#1] SMP PTI
> [ 250.732967] CPU: 2 PID: 38 Comm: kworker/2:1 Not tainted 5.10.0-44-cloud-amd64 #1 Debian 5.10.257-1
> [ 250.751575] Workqueue: events page_reporting_process
> [ 250.756665] RIP: 0010:virtqueue_add_split+0x233/0x4c0 [virtio_ring]
> ...
> [ 250.867678] virtballoon_free_page_report+0x3a/0xe0 [virtio_balloon]
> [ 250.883446] page_reporting_process+0x225/0x4f0
whoops
> Fix this by:
> 1. Unregistering page reporting in virtballoon_freeze() prior to calling
> remove_common(). This clears the RCU pr_dev_info pointer and flushes/
> cancels prdev->work on system_wq via cancel_delayed_work_sync().
> 2. Re-registering page reporting in virtballoon_restore() after the
> virtqueues are re-initialized and virtio_device_ready() has been called.
> 3. Unwinding virtqueue initialization via remove_common() in
> virtballoon_restore() if page_reporting_register() fails.
AI review thinks the patch didn't do the above:
https://sashiko.dev/#/patchset/20260709224330.946683-1-linkl@google.com
It also might have found a couple of pre-existing bugs in there.
^ permalink raw reply
* [RFC] virtio_balloon: fix Use-After-Free in page reporting during PM freeze
From: Link Lin @ 2026-07-09 22:43 UTC (permalink / raw)
To: Andrew Morton, Vlastimil Babka, Michael S . Tsirkin,
David Hildenbrand
Cc: virtualization, linux-mm, linux-kernel, prasin, rientjes, duenwen,
jasowang, xuanzhuo, Ammar Faizi, jiaqiyan, ahwilkins, Greg Thelen,
Alexander Duyck, Link Lin, stable
During system power management freeze (e.g. ACPI S3 suspend or S4
hibernation), virtballoon_freeze() calls remove_common() to reset the
virtio device and delete all virtqueues via vdev->config->del_vqs().
However, unlike virtballoon_remove(), virtballoon_freeze() fails to call
page_reporting_unregister(&vb->pr_dev_info).
The comment in virtballoon_freeze() states:
/*
* The workqueue is already frozen by the PM core before this
* function is called.
*/
While this comment was accurate in 2011 for balloon-internal workqueues
(such as balloon_wq, which was created with WQ_FREEZABLE and is paused
by the PM freezer), it is invalid for Free Page Reporting.
Free Page Reporting (mm/page_reporting.c) schedules its delayed work
(prdev->work) on the global system_wq. Because system_wq lacks the
WQ_FREEZABLE flag, the PM freezer (freeze_workqueues_busy()) explicitly
skips it. Consequently, page_reporting_process() on system_wq remains
active and unfrozen throughout device suspend.
If memory is freed into the buddy allocator or a delayed work timer
expires while the device is being frozen, page_reporting_process() fires
on system_wq and calls virtballoon_free_page_report(). This function
passes vb->reporting_vq into virtqueue_add_inbuf() / virtqueue_add_split().
Because the virtqueues were already destroyed by del_vqs(), this results
in a Use-After-Free / General Protection Fault:
[ 250.709271] general protection fault, probably for non-canonical address 0x7f728084daf08d5e: 0000 [#1] SMP PTI
[ 250.732967] CPU: 2 PID: 38 Comm: kworker/2:1 Not tainted 5.10.0-44-cloud-amd64 #1 Debian 5.10.257-1
[ 250.751575] Workqueue: events page_reporting_process
[ 250.756665] RIP: 0010:virtqueue_add_split+0x233/0x4c0 [virtio_ring]
...
[ 250.867678] virtballoon_free_page_report+0x3a/0xe0 [virtio_balloon]
[ 250.883446] page_reporting_process+0x225/0x4f0
(Note: The OOM Notifier and Shrinker/Free Page Hinting features suffer
from an identical lifecycle flaw and are also vulnerable to UAFs during
S4 hibernation when memory pressure spikes. This patch focuses on Free
Page Reporting, which runs periodically, to ensure clean backports to
stable kernels).
Fix this by:
1. Unregistering page reporting in virtballoon_freeze() prior to calling
remove_common(). This clears the RCU pr_dev_info pointer and flushes/
cancels prdev->work on system_wq via cancel_delayed_work_sync().
2. Re-registering page reporting in virtballoon_restore() after the
virtqueues are re-initialized and virtio_device_ready() has been called.
3. Unwinding virtqueue initialization via remove_common() in
virtballoon_restore() if page_reporting_register() fails.
Fixes: 924a663f75e2 ("virtio-balloon: Reporting free page reservations")
Cc: stable@vger.kernel.org
Cc: jasowang@redhat.com
Cc: xuanzhuo@linux.alibaba.com
Cc: Ammar Faizi <ammarfaizi2@openresty.com>
Cc: jiaqiyan@google.com
Cc: ahwilkins@google.com
Cc: Greg Thelen <gthelen@google.com>
Cc: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Link Lin <linkl@google.com>
---
drivers/virtio/virtio_balloon.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index a1b2c3d4e5f6..45a90fb3abf8 100640
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -1055,6 +1055,9 @@ static int virtballoon_freeze(struct virtio_device *vdev)
* The workqueue is already frozen by the PM core before this
* function is called.
*/
+ if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING))
+ page_reporting_unregister(&vb->pr_dev_info);
+
remove_common(vb);
return 0;
}
static int virtballoon_restore(struct virtio_device *vdev)
{
struct virtio_balloon *vb = vdev->priv;
int ret;
ret = init_vqs(vdev->priv);
if (ret)
return ret;
virtio_device_ready(vdev);
+ if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) {
+ ret = page_reporting_register(&vb->pr_dev_info);
+ if (ret)
+ goto out_remove_vqs;
+ }
+
if (towards_target(vb))
virtballoon_changed(vdev);
update_balloon_size(vb);
return 0;
+
+out_remove_vqs:
+ remove_common(vb);
+ return ret;
}
--
2.45.0
^ permalink raw reply related
* Re: [PATCH net v2 2/2] vsock/test: add test for small packets under pressure
From: Bobby Eshleman @ 2026-07-09 19:48 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez,
Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel,
Michael S. Tsirkin, kvm, Paolo Abeni, virtualization,
Jakub Kicinski, Jason Wang
In-Reply-To: <20260708102904.50732-3-sgarzare@redhat.com>
On Wed, Jul 08, 2026 at 12:29:04PM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
>
> Add a test that sends 2 MB of data using randomly sized small packets
> (129-512 bytes) over a SOCK_STREAM connection. Packets above
> GOOD_COPY_LEN (128) bypass the in-place coalescing in recv_enqueue(),
> forcing each one into its own skb.
>
> Without receive queue collapsing, the per-skb overhead eventually
> exceeds buf_alloc and the connection is reset. The test verifies
> that all data arrives and that content integrity is preserved.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> tools/testing/vsock/vsock_test.c | 87 ++++++++++++++++++++++++++++++++
> 1 file changed, 87 insertions(+)
>
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index 76be0e4a7f0e..b4ff9f946565 100644
> --- a/tools/testing/vsock/vsock_test.c
> +++ b/tools/testing/vsock/vsock_test.c
> @@ -2347,6 +2347,88 @@ static void test_stream_tx_credit_bounds_server(const struct test_opts *opts)
> close(fd);
> }
>
> +/* Test that many small packets don't cause a connection reset under pressure
> + * and that data integrity is preserved. Packet sizes vary randomly between
> + * 129 and 512 bytes, above GOOD_COPY_LEN (128) to bypass in-place coalescing
> + * in recv_enqueue, forcing each one into its own skb. Without receive queue
> + * collapsing, the per-skb overhead eventually exceeds buf_alloc and the
> + * connection is reset.
> + */
> +#define COLLAPSE_PKT_MIN 129
> +#define COLLAPSE_PKT_MAX 512
> +#define COLLAPSE_TOTAL (2 * 1024 * 1024)
> +
> +static void test_stream_collapse_client(const struct test_opts *opts)
> +{
> + unsigned char *data;
> + unsigned long hash;
> + size_t offset = 0;
> + int i, fd;
> +
> + data = malloc(COLLAPSE_TOTAL);
> + if (!data) {
> + perror("malloc");
> + exit(EXIT_FAILURE);
> + }
> +
> + for (i = 0; i < COLLAPSE_TOTAL; i++)
> + data[i] = rand() & 0xff;
> +
> + fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
> + if (fd < 0) {
> + perror("connect");
> + exit(EXIT_FAILURE);
> + }
> +
> + while (offset < COLLAPSE_TOTAL) {
> + size_t pkt_size = COLLAPSE_PKT_MIN +
> + rand() % (COLLAPSE_PKT_MAX - COLLAPSE_PKT_MIN + 1);
> +
> + pkt_size = min(pkt_size, COLLAPSE_TOTAL - offset);
> +
> + send_buf(fd, data + offset, pkt_size, 0, pkt_size);
> + offset += pkt_size;
> + }
> +
> + hash = hash_djb2(data, COLLAPSE_TOTAL);
> + control_writeulong(hash);
> +
> + free(data);
> + close(fd);
> +}
> +
> +static void test_stream_collapse_server(const struct test_opts *opts)
> +{
> + unsigned long hash, remote_hash;
> + unsigned char *data;
> + int fd;
> +
> + data = malloc(COLLAPSE_TOTAL);
> + if (!data) {
> + perror("malloc");
> + exit(EXIT_FAILURE);
> + }
> +
> + fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
> + if (fd < 0) {
> + perror("accept");
> + exit(EXIT_FAILURE);
> + }
> +
> + recv_buf(fd, data, COLLAPSE_TOTAL, 0, COLLAPSE_TOTAL);
> +
> + hash = hash_djb2(data, COLLAPSE_TOTAL);
> + remote_hash = control_readulong();
> + if (hash != remote_hash) {
> + fprintf(stderr, "hash mismatch: local %lu remote %lu\n",
> + hash, remote_hash);
> + exit(EXIT_FAILURE);
> + }
> +
> + free(data);
> + close(fd);
> +}
> +
> static struct test_case test_cases[] = {
> {
> .name = "SOCK_STREAM connection reset",
> @@ -2546,6 +2628,11 @@ static struct test_case test_cases[] = {
> .run_client = test_stream_msg_peek_client,
> .run_server = test_stream_peek_after_recv_server,
> },
> + {
> + .name = "SOCK_STREAM small packets backpressure",
> + .run_client = test_stream_collapse_client,
> + .run_server = test_stream_collapse_server,
> + },
> {},
> };
>
> --
> 2.55.0
>
Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>
^ permalink raw reply
* Re: [PATCH net v2 1/2] vsock/virtio: collapse receive queue under memory pressure
From: Bobby Eshleman @ 2026-07-09 19:07 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez,
Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel,
Michael S. Tsirkin, kvm, Paolo Abeni, virtualization,
Jakub Kicinski, Jason Wang, stable, Brien Oberstein
In-Reply-To: <20260708102904.50732-2-sgarzare@redhat.com>
On Wed, Jul 08, 2026 at 12:29:03PM +0200, 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 proactively from when the number of skb queued
> is close to exceeding the overhead budget.
>
> 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.
>
> Walking a large queue may take a significant amount of time and cache
> misses, causing traffic burstiness. To limit this, the collapse stops
> once enough room is freed for this packet and the next one, but may
> opportunistically free more to fill each collapsed skb to capacity.
>
> [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>
> ---
> v2:
> - defined MAX_COLLAPSE_LEN macro instead of using a variable [Paolo]
> - added a threshold to avoid walking all the queue while collapsing
> [Paolo]
> - collapsed the queue before calling virtio_transport_inc_rx_pkt().
> While working on the threshold, I figured out that the check I was
> introducing can also be used to proactively trigger the collapse, so I
> moved the call to virtio_transport_collapse_rx_queue() before acquiring
> the rx_lock to have also a better diff to simplify backports
> - improved code readability (removed `out` label, `keep` initialization,
> etc.) [Paolo + other small stuff]
> - Brien kindly retested this version as well (thank you so much)
> ---
> net/vmw_vsock/virtio_transport_common.c | 165 +++++++++++++++++++++++-
> 1 file changed, 164 insertions(+), 1 deletion(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 09475007165b..8becad81279c 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -26,6 +26,13 @@
> /* Threshold for detecting small packets to copy */
> #define GOOD_COPY_LEN 128
>
> +/* Max payload that can be collapsed into a single linear skb, using the same
> + * allocation threshold as virtio_vsock_alloc_skb() to avoid adding pressure
> + * on the page allocator.
> + */
> +#define MAX_COLLAPSE_LEN \
> + SKB_MAX_ORDER(VIRTIO_VSOCK_SKB_HEADROOM, PAGE_ALLOC_COSTLY_ORDER)
> +
> static void virtio_transport_cancel_close_work(struct vsock_sock *vsk,
> bool cancel_timeout);
> static s64 virtio_transport_has_space(struct virtio_vsock_sock *vvs);
> @@ -420,6 +427,145 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> return ret;
> }
>
> +static bool virtio_transport_can_collapse(struct sk_buff *skb)
> +{
> + /* 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 >= MAX_COLLAPSE_LEN)
> + 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 target = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
> +
> + while ((skb = skb_peek_next(skb, queue)) &&
> + virtio_transport_can_collapse(skb)) {
> + unsigned int len = skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset;
> +
> + if (len > MAX_COLLAPSE_LEN - target)
> + return target;
> +
> + target += len;
> + }
> +
> + return target;
> +}
> +
> +/* Called under lock_sock to compact the receive queue by merging small skbs.
> + * @min_to_free: minimum number of skbs to eliminate from the queue. May free
> + * more to fill each collapsed skb to capacity.
> + */
> +static void
> +virtio_transport_collapse_rx_queue(struct virtio_vsock_sock *vvs,
> + u32 min_to_free)
> +{
> + struct sk_buff *skb, *next_skb, *new_skb = NULL;
> + struct sk_buff_head new_queue;
> + u32 saved = 0;
> +
> + __skb_queue_head_init(&new_queue);
> +
> + skb_queue_walk_safe(&vvs->rx_queue, skb, next_skb) {
> + 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;
> +
> + keep = !virtio_transport_can_collapse(skb);
> + if (keep) {
> + /* Finalize pending collapsed skb to preserve packet
> + * ordering.
> + */
> + if (new_skb) {
> + __skb_queue_tail(&new_queue, new_skb);
> + new_skb = NULL;
> + saved--;
> + }
> + 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;
> + saved--;
> + }
> +
> + if (!new_skb) {
> + unsigned int alloc_size;
> +
> + /* Check after finalizing to opportunistically fill
> + * each collapsed skb to capacity, merging more skbs
> + * than strictly required.
> + */
> + if (saved >= min_to_free)
> + break;
> +
> + alloc_size = virtio_transport_collapse_size(skb, &vvs->rx_queue);
> +
> + /* Only this skb's data is eligible, nothing to merge
> + * with. Keep as-is.
> + */
> + if (alloc_size <= src_len) {
> + keep = true;
> + goto next;
> + }
> +
> + new_skb = virtio_vsock_alloc_linear_skb(alloc_size +
> + VIRTIO_VSOCK_SKB_HEADROOM, GFP_KERNEL);
> + if (!new_skb)
> + break;
> +
> + memcpy(virtio_vsock_hdr(new_skb), hdr,
> + sizeof(struct virtio_vsock_hdr));
> + virtio_vsock_hdr(new_skb)->len = 0;
> + }
> +
> + /* Cannot fail since src_off/src_len are within bounds, but if
> + * it does, discard new_skb to avoid queuing corrupted data.
> + */
> + if (WARN_ON_ONCE(skb_copy_bits(skb, src_off,
> + skb_put(new_skb, src_len),
> + src_len))) {
> + kfree_skb(new_skb);
> + new_skb = NULL;
> + break;
> + }
> +
> + le32_add_cpu(&virtio_vsock_hdr(new_skb)->len, src_len);
> + virtio_vsock_hdr(new_skb)->flags |= hdr->flags;
> +
> +next:
> + __skb_unlink(skb, &vvs->rx_queue);
> + if (keep) {
> + __skb_queue_tail(&new_queue, skb);
> + } else {
> + consume_skb(skb);
> + saved++;
> + }
> + }
> +
> + if (new_skb)
> + __skb_queue_tail(&new_queue, new_skb);
> +
> + skb_queue_splice(&new_queue, &vvs->rx_queue);
> +}
> +
> static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
> u32 len)
> {
> @@ -1354,12 +1500,29 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> {
> struct virtio_vsock_sock *vvs = vsk->trans;
> bool can_enqueue, free_pkt = false;
> + u32 len, queue_max, queue_len;
> struct virtio_vsock_hdr *hdr;
> - u32 len;
>
> hdr = virtio_vsock_hdr(skb);
> len = le32_to_cpu(hdr->len);
>
> + /* virtio_transport_inc_rx_pkt() rejects packets when the per-skb
> + * overhead (skb_queue_len * SKB_TRUESIZE(0)) exceeds buf_alloc.
> + * Proactively collapse the queue before that happens.
> + * No rx_lock needed: lock_sock is held by caller, preventing
> + * concurrent enqueue or dequeue.
> + */
> + queue_max = vvs->buf_alloc / SKB_TRUESIZE(0);
> + queue_len = skb_queue_len(&vvs->rx_queue);
> + if (queue_len >= queue_max) {
> + /* Walking a large queue may take a significant amount of time
> + * and cache misses, causing traffic burstiness. Limit the
> + * collapse to freeing room for this packet and the next one.
> + * It may free more to fill each collapsed skb to capacity.
> + */
> + virtio_transport_collapse_rx_queue(vvs, queue_len + 2 - queue_max);
> + }
> +
> spin_lock_bh(&vvs->rx_lock);
>
> can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
The changes still look good to me.
Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>
Thanks,
Bobby
^ permalink raw reply
* Re: [PATCH] drm/virtio: Don't detach GEM from a non-created context
From: Dmitry Osipenko @ 2026-07-09 16:23 UTC (permalink / raw)
To: Yiwei Zhang
Cc: Jason Macnak, David Airlie, Gerd Hoffmann, Gurchetan Singh,
dri-devel, virtualization, linux-kernel, stable
In-Reply-To: <CAKT=dD=YecudFK1L9wJ22BO41qXt7V4Qm=nNQQiuwheKejoHdg@mail.gmail.com>
On 7/7/26 22:01, Yiwei Zhang wrote:
> On Mon, Jun 29, 2026 at 2:39 PM Dmitry Osipenko
> <dmitry.osipenko@collabora.com> wrote:
>>
>> Hi,
>>
>> On 6/25/26 20:08, Jason Macnak wrote:
>>> Applies the same treatment as commit 7cf6dd467e87 ("drm/virtio:
>>> Don't attach GEM to a non-created context in gem_object_open()")
>>> to virtio_gpu_gem_object_close() to avoid trying to detach
>>> a resource that was never attached due to a context
>>> never being created when context_init is supported.
>>>
>>> Fixes: 086b9f27f0ab ("drm/virtio: Don't create a context with default param if context_init is supported")
>>> Cc: <stable@vger.kernel.org> # v6.14+
>>> Signed-off-by: Jason Macnak <natsu@google.com>
>>> ---
>>> drivers/gpu/drm/virtio/virtgpu_gem.c | 14 ++++++++------
>>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
>>> index 435d37d36034..66c3f6f74e9c 100644
>>> --- a/drivers/gpu/drm/virtio/virtgpu_gem.c
>>> +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
>>> @@ -139,13 +139,15 @@ void virtio_gpu_gem_object_close(struct drm_gem_object *obj,
>>> if (!vgdev->has_virgl_3d)
>>> return;
>>>
>>> - objs = virtio_gpu_array_alloc(1);
>>> - if (!objs)
>>> - return;
>>> - virtio_gpu_array_add_obj(objs, obj);
>>> + if (vfpriv->context_created) {
>>> + objs = virtio_gpu_array_alloc(1);
>>> + if (!objs)
>>> + return;
>>> + virtio_gpu_array_add_obj(objs, obj);
>>>
>>> - virtio_gpu_cmd_context_detach_resource(vgdev, vfpriv->ctx_id,
>>> - objs);
>>> + virtio_gpu_cmd_context_detach_resource(vgdev, vfpriv->ctx_id,
>>> + objs);
>>> + }
>>> virtio_gpu_notify(vgdev);
>>> }
>>
>> The following scenario still will be troubling:
>>
>> 1. vgdev->has_context_init = true
>> 2. virtio_gpu_gem_object_open() invoked, GEM created and not attached to ctx
>> 3. virtio_gpu_context_init_ioctl() invoked, now vfpriv->context_created
>> = true
>> 4. virtio_gpu_gem_object_close() will detach resource that wasn't attached
>>
>> Add obj->ctx_attached member to struct virtio_gpu_object. See
>> virtio_gpu_object_attach() that uses obj->attached, do the same for
>> virtio_gpu_cmd_context_attach_resource().
>>
>> --
>> Best regards,
>> Dmitry
>
> Hi Dmitry,
>
> WIth context_init, resource attach/detach is per-context based. So a
> simple obj->ctx_attached won't work. One would have to track in the
> guest context_init ctx for whether a bo has been attached or not.
>
> Another option is to accept this patch and live with the case you
> mentioned. We can consider that "invalid" user behavior.
Indeed, obj->ctx_attached shouldn't work for a shared/exported BO. Will
think on it for a couple days more and then merge this version if no
better ideas will appear.
--
Best regards,
Dmitry
^ permalink raw reply
* Re: [PATCH net-next v20 05/12] virtio_net: Query and set flow filter caps
From: Michael S. Tsirkin @ 2026-07-09 14:18 UTC (permalink / raw)
To: Shahar Shitrit
Cc: Daniel Jurgens, netdev, jasowang, pabeni, virtualization, parav,
yohadt, xuanzhuo, eperezma, jgg, kevin.tian, kuba, andrew+netdev,
edumazet
In-Reply-To: <a7f724e5-7648-4362-bfc0-ef4cd87211a8@nvidia.com>
On Thu, Jul 09, 2026 at 04:37:11PM +0300, Shahar Shitrit wrote:
>
>
> On 08/02/2026 13:51, Michael S. Tsirkin wrote:
> > On Thu, Feb 05, 2026 at 04:47:00PM -0600, Daniel Jurgens wrote:
> >> When probing a virtnet device, attempt to read the flow filter
> >> capabilities. In order to use the feature the caps must also
> >> be set. For now setting what was read is sufficient.
> >>
> >> This patch adds uapi definitions virtio_net flow filters define in
> >> version 1.4 of the VirtIO spec.
> >>
> >> Signed-off-by: Daniel Jurgens <danielj@nvidia.com>
> >> Reviewed-by: Parav Pandit <parav@nvidia.com>
> >> Reviewed-by: Shahar Shitrit <shshitrit@nvidia.com>
> >>
> >> ---
> >> v4:
> >> - Validate the length in the selector caps
> >> - Removed __free usage.
> >> - Removed for(int.
> >> v5:
> >> - Remove unneed () after MAX_SEL_LEN macro (test bot)
> >> v6:
> >> - Fix sparse warning "array of flexible structures" Jakub K/Simon H
> >> - Use new variable and validate ff_mask_size before set_cap. MST
> >> v7:
> >> - Set ff->ff_{caps, mask, actions} NULL in error path. Paolo Abeni
> >> - Return errors from virtnet_ff_init, -ENOTSUPP is not fatal. Xuan
> >>
> >> v8:
> >> - Use real_ff_mask_size when setting the selector caps. Jason Wang
> >>
> >> v9:
> >> - Set err after failed memory allocations. Simon Horman
> >>
> >> v10:
> >> - Return -EOPNOTSUPP in virnet_ff_init before allocing any memory.
> >> Jason/Paolo.
> >>
> >> v11:
> >> - Return -EINVAL if any resource limit is 0. Simon Horman
> >> - Ensure we don't overrun alloced space of ff->ff_mask by moving the
> >> real_ff_mask_size > ff_mask_size check into the loop. Simon Horman
> >>
> >> v12:
> >> - Move uapi includes to virtio_net.c vs header file. MST
> >> - Remove kernel.h header in virtio_net_ff uapi. MST
> >> - WARN_ON_ONCE in error paths validating selectors. MST
> >> - Move includes from .h to .c files. MST
> >> - Add WARN_ON_ONCE if obj_destroy fails. MST
> >> - Comment cleanup in virito_net_ff.h uapi. MST
> >> - Add 2 byte pad to the end of virtio_net_ff_cap_data.
> >> https://lore.kernel.org/virtio-comment/20251119044029-mutt-send-email-mst@kernel.org/T/#m930988a5d3db316c68546d8b61f4b94f6ebda030
> >> - Cleanup and reinit in the freeze/restore path. MST
> >>
> >> v13:
> >> - Added /* private: */ comment before reserved field. Jakub
> >> - Change ff_mask validation to break at unkonwn selector type. This
> >> will allow compatability with newer controllers if the types of
> >> selectors is expanded. MST
> >>
> >> v14:
> >> - Handle err from virtnet_ff_init in virtnet_restore_up. MST
> >>
> >> v15:
> >> - In virtnet_restore_up only call virtnet_close in err path if
> >> netif_runnig. AI
> >>
> >> v16:
> >> - Return 0 from virtnet_restore_up if virtnet_init_ff return not
> >> supported. AI
> >>
> >> v17:
> >> - During restore freeze_down on error during ff_init. AI
> >>
> >> v18:
> >> - Changed selector cap validation to verify size for each type
> >> instead of just checking they weren't bigger than max size. AI
> >> - Added __count_by attribute to flexible members in uapi. Paolo A
> >>
> >> v19:
> >> - Fixed ;; and incorrect plural in comment. AI
> >>
> >> v20:
> >> - include uapi/linux/stddef.h for __counted_by. AI
> >
> > AI has led you astray, sadly (
> will fix in v21
> >
> >
> >
> >
> >> ---
> >> drivers/net/virtio_net.c | 231 ++++++++++++++++++++++++++++-
> >> include/uapi/linux/virtio_net_ff.h | 91 ++++++++++++
> >> 2 files changed, 321 insertions(+), 1 deletion(-)
> >> create mode 100644 include/uapi/linux/virtio_net_ff.h
> >>
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index db88dcaefb20..2cfa37e2f83f 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -26,6 +26,11 @@
> >> #include <net/netdev_rx_queue.h>
> >> #include <net/netdev_queues.h>
> >> #include <net/xdp_sock_drv.h>
> >> +#include <linux/virtio_admin.h>
> >> +#include <net/ipv6.h>
> >> +#include <net/ip.h>
> >> +#include <uapi/linux/virtio_pci.h>
> >> +#include <uapi/linux/virtio_net_ff.h>
> >>
> >> static int napi_weight = NAPI_POLL_WEIGHT;
> >> module_param(napi_weight, int, 0444);
> >> @@ -281,6 +286,14 @@ static const struct virtnet_stat_desc virtnet_stats_tx_speed_desc_qstat[] = {
> >> VIRTNET_STATS_DESC_TX_QSTAT(speed, ratelimit_packets, hw_drop_ratelimits),
> >> };
> >>
> >> +struct virtnet_ff {
> >> + struct virtio_device *vdev;
> >> + bool ff_supported;
> >> + struct virtio_net_ff_cap_data *ff_caps;
> >> + struct virtio_net_ff_cap_mask_data *ff_mask;
> >> + struct virtio_net_ff_actions *ff_actions;
> >> +};
> >> +
> >> #define VIRTNET_Q_TYPE_RX 0
> >> #define VIRTNET_Q_TYPE_TX 1
> >> #define VIRTNET_Q_TYPE_CQ 2
> >> @@ -488,6 +501,7 @@ struct virtnet_info {
> >> TRAILING_OVERLAP(struct virtio_net_rss_config_trailer, rss_trailer, hash_key_data,
> >> u8 rss_hash_key_data[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> >> );
> >> + struct virtnet_ff ff;
> >> };
> >> static_assert(offsetof(struct virtnet_info, rss_trailer.hash_key_data) ==
> >> offsetof(struct virtnet_info, rss_hash_key_data));
> >> @@ -526,6 +540,7 @@ static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
> >> struct page *page, void *buf,
> >> int len, int truesize);
> >> static void virtnet_xsk_completed(struct send_queue *sq, int num);
> >> +static void remove_vq_common(struct virtnet_info *vi);
> >>
> >> enum virtnet_xmit_type {
> >> VIRTNET_XMIT_TYPE_SKB,
> >> @@ -5684,6 +5699,192 @@ static const struct netdev_stat_ops virtnet_stat_ops = {
> >> .get_base_stats = virtnet_get_base_stats,
> >> };
> >>
> >> +static size_t get_mask_size(u16 type)
> >> +{
> >> + switch (type) {
> >> + case VIRTIO_NET_FF_MASK_TYPE_ETH:
> >> + return sizeof(struct ethhdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_IPV4:
> >> + return sizeof(struct iphdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_IPV6:
> >> + return sizeof(struct ipv6hdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_TCP:
> >> + return sizeof(struct tcphdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_UDP:
> >> + return sizeof(struct udphdr);
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int virtnet_ff_init(struct virtnet_ff *ff, struct virtio_device *vdev)
> >> +{
> >> + size_t ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data) +
> >> + sizeof(struct virtio_net_ff_selector) *
> >> + VIRTIO_NET_FF_MASK_TYPE_MAX;
> >> + struct virtio_admin_cmd_query_cap_id_result *cap_id_list;
> >> + struct virtio_net_ff_selector *sel;
> >> + unsigned long sel_types = 0;
> >> + size_t real_ff_mask_size;
> >> + int err;
> >> + int i;
> >> +
> >> + if (!vdev->config->admin_cmd_exec)
> >> + return -EOPNOTSUPP;
> >> +
> >> + cap_id_list = kzalloc(sizeof(*cap_id_list), GFP_KERNEL);
> >> + if (!cap_id_list)
> >> + return -ENOMEM;
> >> +
> >> + err = virtio_admin_cap_id_list_query(vdev, cap_id_list);
> >> + if (err)
> >> + goto err_cap_list;
> >> +
> >> + if (!(VIRTIO_CAP_IN_LIST(cap_id_list,
> >> + VIRTIO_NET_FF_RESOURCE_CAP) &&
> >> + VIRTIO_CAP_IN_LIST(cap_id_list,
> >> + VIRTIO_NET_FF_SELECTOR_CAP) &&
> >> + VIRTIO_CAP_IN_LIST(cap_id_list,
> >> + VIRTIO_NET_FF_ACTION_CAP))) {
> >> + err = -EOPNOTSUPP;
> >> + goto err_cap_list;
> >> + }
> >> +
> >> + ff->ff_caps = kzalloc(sizeof(*ff->ff_caps), GFP_KERNEL);
> >> + if (!ff->ff_caps) {
> >> + err = -ENOMEM;
> >> + goto err_cap_list;
> >> + }
> >> +
> >> + err = virtio_admin_cap_get(vdev,
> >> + VIRTIO_NET_FF_RESOURCE_CAP,
> >> + ff->ff_caps,
> >> + sizeof(*ff->ff_caps));
> >> +
> >> + if (err)
> >> + goto err_ff;
> >> +
> >> + if (!ff->ff_caps->groups_limit ||
> >> + !ff->ff_caps->classifiers_limit ||
> >> + !ff->ff_caps->rules_limit ||
> >> + !ff->ff_caps->rules_per_group_limit) {
> >> + err = -EINVAL;
> >> + goto err_ff;
> >> + }
> >> +
> >> + /* VIRTIO_NET_FF_MASK_TYPE start at 1 */
> >> + for (i = 1; i <= VIRTIO_NET_FF_MASK_TYPE_MAX; i++)
> >> + ff_mask_size += get_mask_size(i);
> >> +
> >> + ff->ff_mask = kzalloc(ff_mask_size, GFP_KERNEL);
> >> + if (!ff->ff_mask) {
> >> + err = -ENOMEM;
> >> + goto err_ff;
> >> + }
> >> +
> >> + err = virtio_admin_cap_get(vdev,
> >> + VIRTIO_NET_FF_SELECTOR_CAP,
> >> + ff->ff_mask,
> >> + ff_mask_size);
> >
> > So ff_actions is from device and ff_actions->count does not seem to be checked.
> >
> > If device somehow gains a larger mask down the road, can it not then overflow?
> > or malicious?
> see below
> >
> >
> >> +
> >> + if (err)
> >> + goto err_ff_mask;
> >> +
> >> + ff->ff_actions = kzalloc(sizeof(*ff->ff_actions) +
> >> + VIRTIO_NET_FF_ACTION_MAX,
> >> + GFP_KERNEL);
> >> + if (!ff->ff_actions) {
> >> + err = -ENOMEM;
> >> + goto err_ff_mask;
> >> + }
> >> +
> >> + err = virtio_admin_cap_get(vdev,
> >> + VIRTIO_NET_FF_ACTION_CAP,
> >> + ff->ff_actions,
> >> + sizeof(*ff->ff_actions) + VIRTIO_NET_FF_ACTION_MAX);
> >
> > So ff_actions is from device and ff_actions->count is not checked.
> >
> > If device gains a ton of actions down the road, can it not then overflow?
> > or malicious?
>
> it can't overflow, as it can write up to sizeof(*ff->ff_actions) +
> VIRTIO_NET_FF_ACTION_MAX bytes. But it is a valid concern in case of
> someone in the future iterates on count. Better to handle it now so it
> won't be forgotten. Checking that ff_actions->count doesn't exceed
> VIRTIO_NET_FF_ACTION_MAX can break backward compatibility.
These are admin commands right? They are truncated silently.
All you have to do is just check the length returned.
At least that's my vague understanding, don't have the
time to re-review now. HTH
> If this max
> value is extended in a new spec/controller then this driver would fail
> to load. Instead, I will assign ff->ff_actions->count with the minimum
> of what we got and the max.
> >
> >> +
> >> + if (err)
> >> + goto err_ff_action;
> >> +
> >> + err = virtio_admin_cap_set(vdev,
> >> + VIRTIO_NET_FF_RESOURCE_CAP,
> >> + ff->ff_caps,
> >> + sizeof(*ff->ff_caps));
> >> + if (err)
> >> + goto err_ff_action;
> >> +
> >> + real_ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data);
> >> + sel = (void *)&ff->ff_mask->selectors;
> >> +
> >> + for (i = 0; i < ff->ff_mask->count; i++) {
> >> + /* If the selector type is unknown it may indicate the spec
> >> + * has been revised to include new types of selectors
> >> + */
> >> + if (sel->type > VIRTIO_NET_FF_MASK_TYPE_MAX)
> >
> > do you want to check sel->type 0 too?
> yes, will add in v21.
> >
> >> + break;
> >
> > but count remains unchanged? should we not to reduce count here
> > so device knows what driver can drive?
>
> we should. will add it in v21.
> >
> >
> >> +
> >> + if (sel->length != get_mask_size(sel->type) ||
> >> + test_and_set_bit(sel->type, &sel_types)) {
> >> + WARN_ON_ONCE(true);
> >> + err = -EINVAL;
> >> + goto err_ff_action;
> >> + }
> >> + real_ff_mask_size += sizeof(struct virtio_net_ff_selector) + sel->length;
> >> + if (real_ff_mask_size > ff_mask_size) {
> >> + WARN_ON_ONCE(true);
> >> + err = -EINVAL;
> >> + goto err_ff_action;
> >> + }
> >> + sel = (void *)sel + sizeof(*sel) + sel->length;
> >> + }
> >> +
> >> + err = virtio_admin_cap_set(vdev,
> >> + VIRTIO_NET_FF_SELECTOR_CAP,
> >> + ff->ff_mask,
> >> + real_ff_mask_size);
> >> + if (err)
> >> + goto err_ff_action;
> >> +
> >> + err = virtio_admin_cap_set(vdev,
> >> + VIRTIO_NET_FF_ACTION_CAP,
> >> + ff->ff_actions,
> >> + sizeof(*ff->ff_actions) + VIRTIO_NET_FF_ACTION_MAX);
> >> + if (err)
> >> + goto err_ff_action;
> >> +
> >> + ff->vdev = vdev;
> >> + ff->ff_supported = true;
> >> +
> >> + kfree(cap_id_list);
> >> +
> >> + return 0;
> >> +
> >> +err_ff_action:
> >> + kfree(ff->ff_actions);
> >> + ff->ff_actions = NULL;
> >> +err_ff_mask:
> >> + kfree(ff->ff_mask);
> >> + ff->ff_mask = NULL;
> >> +err_ff:
> >> + kfree(ff->ff_caps);
> >> + ff->ff_caps = NULL;
> >> +err_cap_list:
> >> + kfree(cap_id_list);
> >> +
> >> + return err;
> >> +}
> >> +
> >> +static void virtnet_ff_cleanup(struct virtnet_ff *ff)
> >> +{
> >> + if (!ff->ff_supported)
> >> + return;
> >> +
> >> + kfree(ff->ff_actions);
> >> + kfree(ff->ff_mask);
> >> + kfree(ff->ff_caps);
> >> + ff->ff_supported = false;
> >> +}
> >> +
> >> static void virtnet_freeze_down(struct virtio_device *vdev)
> >> {
> >> struct virtnet_info *vi = vdev->priv;
> >> @@ -5702,6 +5903,10 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
> >> netif_tx_lock_bh(vi->dev);
> >> netif_device_detach(vi->dev);
> >> netif_tx_unlock_bh(vi->dev);
> >> +
> >> + rtnl_lock();
> >> + virtnet_ff_cleanup(&vi->ff);
> >> + rtnl_unlock();
> >> }
> >>
> >> static int init_vqs(struct virtnet_info *vi);
> >> @@ -5727,10 +5932,23 @@ static int virtnet_restore_up(struct virtio_device *vdev)
> >> return err;
> >> }
> >>
> >> + /* Initialize flow filters. Not supported is an acceptable and common
> >> + * return code
> >> + */
> >> + rtnl_lock();
> >> + err = virtnet_ff_init(&vi->ff, vi->vdev);
> >> + if (err && err != -EOPNOTSUPP) {
> >> + rtnl_unlock();
> >> + virtnet_freeze_down(vi->vdev);
> >> + remove_vq_common(vi);
> >> + return err;
> >> + }
> >> + rtnl_unlock();
> >> +
> >> netif_tx_lock_bh(vi->dev);
> >> netif_device_attach(vi->dev);
> >> netif_tx_unlock_bh(vi->dev);
> >> - return err;
> >> + return 0;
> >> }
> >>
> >> static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64 offloads)
> >> @@ -7058,6 +7276,15 @@ static int virtnet_probe(struct virtio_device *vdev)
> >> }
> >> vi->guest_offloads_capable = vi->guest_offloads;
> >>
> >> + /* Initialize flow filters. Not supported is an acceptable and common
> >> + * return code
> >> + */
> >> + err = virtnet_ff_init(&vi->ff, vi->vdev);
> >> + if (err && err != -EOPNOTSUPP) {
> >> + rtnl_unlock();
> >> + goto free_unregister_netdev;
> >> + }
> >> +
> >> rtnl_unlock();
> >>
> >> err = virtnet_cpu_notif_add(vi);
> >> @@ -7073,6 +7300,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> >>
> >> free_unregister_netdev:
> >> unregister_netdev(dev);
> >> + virtnet_ff_cleanup(&vi->ff);
> >> free_failover:
> >> net_failover_destroy(vi->failover);
> >> free_vqs:
> >> @@ -7121,6 +7349,7 @@ static void virtnet_remove(struct virtio_device *vdev)
> >> virtnet_free_irq_moder(vi);
> >>
> >> unregister_netdev(vi->dev);
> >> + virtnet_ff_cleanup(&vi->ff);
> >>
> >> net_failover_destroy(vi->failover);
> >>
> >> diff --git a/include/uapi/linux/virtio_net_ff.h b/include/uapi/linux/virtio_net_ff.h
> >> new file mode 100644
> >> index 000000000000..552a6b3a8a91
> >> --- /dev/null
> >> +++ b/include/uapi/linux/virtio_net_ff.h
> >> @@ -0,0 +1,91 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
> >> + *
> >> + * Header file for virtio_net flow filters
> >> + */
> >> +#ifndef _LINUX_VIRTIO_NET_FF_H
> >> +#define _LINUX_VIRTIO_NET_FF_H
> >> +
> >> +#include <linux/types.h>
> >> +#include <uapi/linux/stddef.h>
> >> +
> >> +#define VIRTIO_NET_FF_RESOURCE_CAP 0x800
> >> +#define VIRTIO_NET_FF_SELECTOR_CAP 0x801
> >> +#define VIRTIO_NET_FF_ACTION_CAP 0x802
> >> +
> >> +/**
> >> + * struct virtio_net_ff_cap_data - Flow filter resource capability limits
> >> + * @groups_limit: maximum number of flow filter groups supported by the device
> >> + * @classifiers_limit: maximum number of classifiers supported by the device
> >> + * @rules_limit: maximum number of rules supported device-wide across all groups
> >> + * @rules_per_group_limit: maximum number of rules allowed in a single group
> >> + * @last_rule_priority: priority value associated with the lowest-priority rule
> >> + * @selectors_per_classifier_limit: maximum selectors allowed in one classifier
> >> + */
> >> +struct virtio_net_ff_cap_data {
> >> + __le32 groups_limit;
> >> + __le32 classifiers_limit;
> >> + __le32 rules_limit;
> >> + __le32 rules_per_group_limit;
> >> + __u8 last_rule_priority;
> >> + __u8 selectors_per_classifier_limit;
> >> + /* private: */
> >> + __u8 reserved[2];
> >> +};
> >> +
> >> +/**
> >> + * struct virtio_net_ff_selector - Selector mask descriptor
> >> + * @type: selector type, one of VIRTIO_NET_FF_MASK_TYPE_* constants
> >> + * @flags: selector flags, see VIRTIO_NET_FF_MASK_F_* constants
> >> + * @reserved: must be set to 0 by the driver and ignored by the device
> >> + * @length: size in bytes of @mask
> >> + * @reserved1: must be set to 0 by the driver and ignored by the device
> >> + * @mask: variable-length mask payload for @type, length given by @length
> >> + *
> >> + * A selector describes a header mask that a classifier can apply. The format
> >> + * of @mask depends on @type.
> >> + */
> >> +struct virtio_net_ff_selector {
> >> + __u8 type;
> >> + __u8 flags;
> >> + __u8 reserved[2];
> >> + __u8 length;
> >> + __u8 reserved1[3];
> >> + __u8 mask[] __counted_by(length);
> >> +};
> >> +
> >> +#define VIRTIO_NET_FF_MASK_TYPE_ETH 1
> >> +#define VIRTIO_NET_FF_MASK_TYPE_IPV4 2
> >> +#define VIRTIO_NET_FF_MASK_TYPE_IPV6 3
> >> +#define VIRTIO_NET_FF_MASK_TYPE_TCP 4
> >> +#define VIRTIO_NET_FF_MASK_TYPE_UDP 5
> >> +#define VIRTIO_NET_FF_MASK_TYPE_MAX VIRTIO_NET_FF_MASK_TYPE_UDP
> >> +
> >> +/**
> >> + * struct virtio_net_ff_cap_mask_data - Supported selector mask formats
> >> + * @count: number of entries in @selectors
> >> + * @reserved: must be set to 0 by the driver and ignored by the device
> >> + * @selectors: packed array of struct virtio_net_ff_selector.
> >> + */
> >> +struct virtio_net_ff_cap_mask_data {
> >> + __u8 count;
> >> + __u8 reserved[7];
> >> + __u8 selectors[] __counted_by(count);
> >
> > This looks wrong to me. count is # of selectors (packed entries) not
> > bytes.
> right, will remove it.
> > >
> >
> >
> >> +};
> >> +
> >> +#define VIRTIO_NET_FF_MASK_F_PARTIAL_MASK (1 << 0)
> >> +
> >> +#define VIRTIO_NET_FF_ACTION_DROP 1
> >> +#define VIRTIO_NET_FF_ACTION_RX_VQ 2
> >> +#define VIRTIO_NET_FF_ACTION_MAX VIRTIO_NET_FF_ACTION_RX_VQ
> >> +/**
> >> + * struct virtio_net_ff_actions - Supported flow actions
> >> + * @count: number of supported actions in @actions
> >> + * @reserved: must be set to 0 by the driver and ignored by the device
> >> + * @actions: array of action identifiers (VIRTIO_NET_FF_ACTION_*)
> >> + */
> >> +struct virtio_net_ff_actions {
> >> + __u8 count;
> >> + __u8 reserved[7];
> >> + __u8 actions[] __counted_by(count);
> >
> >
> > this too.
> count directly represents the number of __u8 elements in actions[], so
> the size of the flexible array is count * sizeof(__u8). Therefore, here
> it is correct.
> >
> >> +};
> >> +#endif
> >> --
> >> 2.50.1
> >
> >
^ permalink raw reply
* Re: [PATCH net-next v20 07/12] virtio_net: Implement layer 2 ethtool flow rules
From: Shahar Shitrit @ 2026-07-09 13:44 UTC (permalink / raw)
To: Michael S. Tsirkin, Daniel Jurgens
Cc: netdev, jasowang, pabeni, virtualization, parav, yohadt, xuanzhuo,
eperezma, jgg, kevin.tian, kuba, andrew+netdev, edumazet
In-Reply-To: <20260208062637-mutt-send-email-mst@kernel.org>
On 08/02/2026 13:35, Michael S. Tsirkin wrote:
> On Thu, Feb 05, 2026 at 04:47:02PM -0600, Daniel Jurgens wrote:
>> Filtering a flow requires a classifier to match the packets, and a rule
>> to filter on the matches.
>>
>> A classifier consists of one or more selectors. There is one selector
>> per header type. A selector must only use fields set in the selector
>> capability. If partial matching is supported, the classifier mask for a
>> particular field can be a subset of the mask for that field in the
>> capability.
>>
>> The rule consists of a priority, an action and a key. The key is a byte
>> array containing headers corresponding to the selectors in the
>> classifier.
>>
>> This patch implements ethtool rules for ethernet headers.
>>
>> Example:
>> $ ethtool -U ens9 flow-type ether dst 08:11:22:33:44:54 action 30
>> Added rule with ID 1
>>
>> The rule in the example directs received packets with the specified
>> destination MAC address to rq 30.
>>
>> Signed-off-by: Daniel Jurgens <danielj@nvidia.com>
>> Reviewed-by: Parav Pandit <parav@nvidia.com>
>> Reviewed-by: Shahar Shitrit <shshitrit@nvidia.com>
>> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> ---
>> v4:
>> - Fixed double free bug in error flows
>> - Build bug on for classifier struct ordering.
>> - (u8 *) to (void *) casting.
>> - Documentation in UAPI
>> - Answered questions about overflow with no changes.
>> v6:
>> - Fix sparse warning "array of flexible structures" Jakub K/Simon H
>> v7:
>> - Move for (int i -> for (i hunk from next patch. Paolo Abeni
>>
>> v12:
>> - Make key_size u8. MST
>> - Free key in insert_rule when it's successful. MST
>>
>> v17:
>> - Fix memory leak if validate_classifier_selector fails. AI
>>
>> ---
>> ---
>> drivers/net/virtio_net.c | 464 +++++++++++++++++++++++++++++
>> include/uapi/linux/virtio_net_ff.h | 50 ++++
>> 2 files changed, 514 insertions(+)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 340104b22a59..27833ba1abee 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -31,6 +31,7 @@
>> #include <net/ip.h>
>> #include <uapi/linux/virtio_pci.h>
>> #include <uapi/linux/virtio_net_ff.h>
>> +#include <linux/xarray.h>
>>
>> static int napi_weight = NAPI_POLL_WEIGHT;
>> module_param(napi_weight, int, 0444);
>> @@ -286,6 +287,11 @@ static const struct virtnet_stat_desc virtnet_stats_tx_speed_desc_qstat[] = {
>> VIRTNET_STATS_DESC_TX_QSTAT(speed, ratelimit_packets, hw_drop_ratelimits),
>> };
>>
>> +struct virtnet_ethtool_ff {
>> + struct xarray rules;
>> + int num_rules;
>> +};
>> +
>> #define VIRTNET_FF_ETHTOOL_GROUP_PRIORITY 1
>> #define VIRTNET_FF_MAX_GROUPS 1
>>
>> @@ -295,8 +301,16 @@ struct virtnet_ff {
>> struct virtio_net_ff_cap_data *ff_caps;
>> struct virtio_net_ff_cap_mask_data *ff_mask;
>> struct virtio_net_ff_actions *ff_actions;
>> + struct xarray classifiers;
>> + int num_classifiers;
>> + struct virtnet_ethtool_ff ethtool;
>> };
>>
>> +static int virtnet_ethtool_flow_insert(struct virtnet_ff *ff,
>> + struct ethtool_rx_flow_spec *fs,
>> + u16 curr_queue_pairs);
>> +static int virtnet_ethtool_flow_remove(struct virtnet_ff *ff, int location);
>> +
>> #define VIRTNET_Q_TYPE_RX 0
>> #define VIRTNET_Q_TYPE_TX 1
>> #define VIRTNET_Q_TYPE_CQ 2
>> @@ -5579,6 +5593,21 @@ static u32 virtnet_get_rx_ring_count(struct net_device *dev)
>> return vi->curr_queue_pairs;
>> }
>>
>> +static int virtnet_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info)
>> +{
>> + struct virtnet_info *vi = netdev_priv(dev);
>> +
>> + switch (info->cmd) {
>> + case ETHTOOL_SRXCLSRLINS:
>> + return virtnet_ethtool_flow_insert(&vi->ff, &info->fs,
>> + vi->curr_queue_pairs);
>> + case ETHTOOL_SRXCLSRLDEL:
>> + return virtnet_ethtool_flow_remove(&vi->ff, info->fs.location);
>> + }
>> +
>> + return -EOPNOTSUPP;
>> +}
>> +
>> static const struct ethtool_ops virtnet_ethtool_ops = {
>> .supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES |
>> ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_USE_ADAPTIVE_RX,
>> @@ -5605,6 +5634,7 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>> .get_rxfh_fields = virtnet_get_hashflow,
>> .set_rxfh_fields = virtnet_set_hashflow,
>> .get_rx_ring_count = virtnet_get_rx_ring_count,
>> + .set_rxnfc = virtnet_set_rxnfc,
>> };
>>
>> static void virtnet_get_queue_stats_rx(struct net_device *dev, int i,
>> @@ -5702,6 +5732,429 @@ static const struct netdev_stat_ops virtnet_stat_ops = {
>> .get_base_stats = virtnet_get_base_stats,
>> };
>>
>> +struct virtnet_ethtool_rule {
>> + struct ethtool_rx_flow_spec flow_spec;
>> + u32 classifier_id;
>> +};
>> +
>> +/* The classifier struct must be the last field in this struct */
>> +struct virtnet_classifier {
>> + size_t size;
>> + u32 id;
>> + struct virtio_net_resource_obj_ff_classifier classifier;
>> +};
>> +
>> +static_assert(sizeof(struct virtnet_classifier) ==
>> + ALIGN(offsetofend(struct virtnet_classifier, classifier),
>> + __alignof__(struct virtnet_classifier)),
>> + "virtnet_classifier: classifier must be the last member");
>> +
>> +static bool check_mask_vs_cap(const void *m, const void *c,
>> + u16 len, bool partial)
>> +{
>> + const u8 *mask = m;
>> + const u8 *cap = c;
>> + int i;
>> +
>> + for (i = 0; i < len; i++) {
>> + if (partial && ((mask[i] & cap[i]) != mask[i]))
>> + return false;
>> + if (!partial && mask[i] != cap[i])
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static
>> +struct virtio_net_ff_selector *get_selector_cap(const struct virtnet_ff *ff,
>> + u8 selector_type)
>> +{
>> + struct virtio_net_ff_selector *sel;
>> + void *buf;
>> + int i;
>> +
>> + buf = &ff->ff_mask->selectors;
>> + sel = buf;
>> +
>> + for (i = 0; i < ff->ff_mask->count; i++) {
>> + if (sel->type == selector_type)
>> + return sel;
>> +
>> + buf += sizeof(struct virtio_net_ff_selector) + sel->length;
>> + sel = buf;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static bool validate_eth_mask(const struct virtnet_ff *ff,
>> + const struct virtio_net_ff_selector *sel,
>> + const struct virtio_net_ff_selector *sel_cap)
>> +{
>> + bool partial_mask = !!(sel_cap->flags & VIRTIO_NET_FF_MASK_F_PARTIAL_MASK);
>> + struct ethhdr *cap, *mask;
>> + struct ethhdr zeros = {};
>> +
>> + cap = (struct ethhdr *)&sel_cap->mask;
>> + mask = (struct ethhdr *)&sel->mask;
>> +
>> + if (memcmp(&zeros.h_dest, mask->h_dest, sizeof(zeros.h_dest)) &&
>> + !check_mask_vs_cap(mask->h_dest, cap->h_dest,
>> + sizeof(mask->h_dest), partial_mask))
>> + return false;
>> +
>> + if (memcmp(&zeros.h_source, mask->h_source, sizeof(zeros.h_source)) &&
>> + !check_mask_vs_cap(mask->h_source, cap->h_source,
>> + sizeof(mask->h_source), partial_mask))
>> + return false;
>> +
>> + if (mask->h_proto &&
>> + !check_mask_vs_cap(&mask->h_proto, &cap->h_proto,
>> + sizeof(__be16), partial_mask))
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +static bool validate_mask(const struct virtnet_ff *ff,
>> + const struct virtio_net_ff_selector *sel)
>> +{
>> + struct virtio_net_ff_selector *sel_cap = get_selector_cap(ff, sel->type);
>> +
>> + if (!sel_cap)
>> + return false;
>> +
>> + switch (sel->type) {
>> + case VIRTIO_NET_FF_MASK_TYPE_ETH:
>> + return validate_eth_mask(ff, sel, sel_cap);
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static int setup_classifier(struct virtnet_ff *ff, struct virtnet_classifier *c)
>> +{
>> + int err;
>> +
>> + err = xa_alloc(&ff->classifiers, &c->id, c,
>> + XA_LIMIT(0, le32_to_cpu(ff->ff_caps->classifiers_limit) - 1),
>> + GFP_KERNEL);
>> + if (err)
>> + return err;
>> +
>> + err = virtio_admin_obj_create(ff->vdev,
>> + VIRTIO_NET_RESOURCE_OBJ_FF_CLASSIFIER,
>> + c->id,
>> + VIRTIO_ADMIN_GROUP_TYPE_SELF,
>> + 0,
>> + &c->classifier,
>> + c->size);
>> + if (err)
>> + goto err_xarray;
>> +
>> + return 0;
>> +
>> +err_xarray:
>> + xa_erase(&ff->classifiers, c->id);
>> +
>> + return err;
>> +}
>> +
>> +static void destroy_classifier(struct virtnet_ff *ff,
>> + u32 classifier_id)
>> +{
>> + struct virtnet_classifier *c;
>> +
>> + c = xa_load(&ff->classifiers, classifier_id);
>> + if (c) {
>> + virtio_admin_obj_destroy(ff->vdev,
>> + VIRTIO_NET_RESOURCE_OBJ_FF_CLASSIFIER,
>> + c->id,
>> + VIRTIO_ADMIN_GROUP_TYPE_SELF,
>> + 0);
>> +
>> + xa_erase(&ff->classifiers, c->id);
>> + kfree(c);
>> + }
>> +}
>> +
>> +static void destroy_ethtool_rule(struct virtnet_ff *ff,
>> + struct virtnet_ethtool_rule *eth_rule)
>> +{
>> + ff->ethtool.num_rules--;
>> +
>> + virtio_admin_obj_destroy(ff->vdev,
>> + VIRTIO_NET_RESOURCE_OBJ_FF_RULE,
>> + eth_rule->flow_spec.location,
>> + VIRTIO_ADMIN_GROUP_TYPE_SELF,
>> + 0);
>> +
>> + xa_erase(&ff->ethtool.rules, eth_rule->flow_spec.location);
>> + destroy_classifier(ff, eth_rule->classifier_id);
>> + kfree(eth_rule);
>> +}
>> +
>> +static int insert_rule(struct virtnet_ff *ff,
>> + struct virtnet_ethtool_rule *eth_rule,
>> + u32 classifier_id,
>> + const u8 *key,
>> + u8 key_size)
>> +{
>> + struct ethtool_rx_flow_spec *fs = ð_rule->flow_spec;
>> + struct virtio_net_resource_obj_ff_rule *ff_rule;
>> + int err;
>> +
>> + ff_rule = kzalloc(sizeof(*ff_rule) + key_size, GFP_KERNEL);
>> + if (!ff_rule)
>> + return -ENOMEM;
>> +
>> + /* Intentionally leave the priority as 0. All rules have the same
>> + * priority.
>> + */
>> + ff_rule->group_id = cpu_to_le32(VIRTNET_FF_ETHTOOL_GROUP_PRIORITY);
>> + ff_rule->classifier_id = cpu_to_le32(classifier_id);
>> + ff_rule->key_length = key_size;
>> + ff_rule->action = fs->ring_cookie == RX_CLS_FLOW_DISC ?
>> + VIRTIO_NET_FF_ACTION_DROP :
>> + VIRTIO_NET_FF_ACTION_RX_VQ;
>
> btw driver does not validate these actions are supported or did i miss the
> check? the spec does not say it must but it also does not
> say how device will behave if not. better not to take risks.
>
>
>> + ff_rule->vq_index = fs->ring_cookie != RX_CLS_FLOW_DISC ?
>> + cpu_to_le16(fs->ring_cookie) : 0;
>
>
> So here ring_cookie is a vq index? vq index is what matches the spec. but below ...
here we miss rxq2vq conversion of fs->ring_cookie. I'll fix in v21.
>
>
>> + memcpy(&ff_rule->keys, key, key_size);
>> +
>> + err = virtio_admin_obj_create(ff->vdev,
>> + VIRTIO_NET_RESOURCE_OBJ_FF_RULE,
>> + fs->location,
>> + VIRTIO_ADMIN_GROUP_TYPE_SELF,
>> + 0,
>> + ff_rule,
>> + sizeof(*ff_rule) + key_size);
>> + if (err)
>> + goto err_ff_rule;
>> +
>> + eth_rule->classifier_id = classifier_id;
>> + ff->ethtool.num_rules++;
>> + kfree(ff_rule);
>> + kfree(key);
>> +
>> + return 0;
>> +
>> +err_ff_rule:
>> + kfree(ff_rule);
>> +
>> + return err;
>> +}
>> +
>> +static u32 flow_type_mask(u32 flow_type)
>> +{
>> + return flow_type & ~(FLOW_EXT | FLOW_MAC_EXT | FLOW_RSS);
>> +}
>> +
>> +static bool supported_flow_type(const struct ethtool_rx_flow_spec *fs)
>> +{
>> + switch (fs->flow_type) {
>> + case ETHER_FLOW:
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static int validate_flow_input(struct virtnet_ff *ff,
>> + const struct ethtool_rx_flow_spec *fs,
>> + u16 curr_queue_pairs)
>> +{
>> + /* Force users to use RX_CLS_LOC_ANY - don't allow specific locations */
>> + if (fs->location != RX_CLS_LOC_ANY)
>> + return -EOPNOTSUPP;
>> +
>> + if (fs->ring_cookie != RX_CLS_FLOW_DISC &&
>> + fs->ring_cookie >= curr_queue_pairs)
>> + return -EINVAL;
>
>
> here ring cookie seems to be a queue pair index?
>
>> +
>> + if (fs->flow_type != flow_type_mask(fs->flow_type))
>> + return -EOPNOTSUPP;
>> +
>> + if (!supported_flow_type(fs))
>> + return -EOPNOTSUPP;
>> +
>> + return 0;
>> +}
>> +
>> +static void calculate_flow_sizes(struct ethtool_rx_flow_spec *fs,
>> + u8 *key_size, size_t *classifier_size,
>> + int *num_hdrs)
>> +{
>> + *num_hdrs = 1;
>> + *key_size = sizeof(struct ethhdr);
>> + /*
>> + * The classifier size is the size of the classifier header, a selector
>> + * header for each type of header in the match criteria, and each header
>> + * providing the mask for matching against.
>> + */
>> + *classifier_size = *key_size +
>> + sizeof(struct virtio_net_resource_obj_ff_classifier) +
>> + sizeof(struct virtio_net_ff_selector) * (*num_hdrs);
>> +}
>> +
>> +static void setup_eth_hdr_key_mask(struct virtio_net_ff_selector *selector,
>> + u8 *key,
>> + const struct ethtool_rx_flow_spec *fs)
>> +{
>> + struct ethhdr *eth_m = (struct ethhdr *)&selector->mask;
>> + struct ethhdr *eth_k = (struct ethhdr *)key;
>> +
>> + selector->type = VIRTIO_NET_FF_MASK_TYPE_ETH;
>> + selector->length = sizeof(struct ethhdr);
>> +
>> + memcpy(eth_m, &fs->m_u.ether_spec, sizeof(*eth_m));
>> + memcpy(eth_k, &fs->h_u.ether_spec, sizeof(*eth_k));
>> +}
>> +
>> +static int
>> +validate_classifier_selectors(struct virtnet_ff *ff,
>> + struct virtio_net_resource_obj_ff_classifier *classifier,
>> + int num_hdrs)
>> +{
>> + struct virtio_net_ff_selector *selector = (void *)classifier->selectors;
>> + int i;
>> +
>> + for (i = 0; i < num_hdrs; i++) {
>> + if (!validate_mask(ff, selector))
>> + return -EINVAL;
>> +
>> + selector = (((void *)selector) + sizeof(*selector) +
>> + selector->length);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int build_and_insert(struct virtnet_ff *ff,
>> + struct virtnet_ethtool_rule *eth_rule)
>> +{
>> + struct virtio_net_resource_obj_ff_classifier *classifier;
>> + struct ethtool_rx_flow_spec *fs = ð_rule->flow_spec;
>> + struct virtio_net_ff_selector *selector;
>> + struct virtnet_classifier *c;
>> + size_t classifier_size;
>> + int num_hdrs;
>> + u8 key_size;
>> + u8 *key;
>> + int err;
>> +
>> + calculate_flow_sizes(fs, &key_size, &classifier_size, &num_hdrs);
>> +
>> + key = kzalloc(key_size, GFP_KERNEL);
>> + if (!key)
>> + return -ENOMEM;
>> +
>> + /*
>> + * virtio_net_ff_obj_ff_classifier is already included in the
>> + * classifier_size.
>> + */
>> + c = kzalloc(classifier_size +
>> + sizeof(struct virtnet_classifier) -
>> + sizeof(struct virtio_net_resource_obj_ff_classifier),
>> + GFP_KERNEL);
>> + if (!c) {
>> + kfree(key);
>> + return -ENOMEM;
>> + }
>> +
>> + c->size = classifier_size;
>> + classifier = &c->classifier;
>> + classifier->count = num_hdrs;
>> + selector = (void *)&classifier->selectors[0];
>> +
>> + setup_eth_hdr_key_mask(selector, key, fs);
>> +
>> + err = validate_classifier_selectors(ff, classifier, num_hdrs);
>> + if (err)
>> + goto err_classifier;
>> +
>> + err = setup_classifier(ff, c);
>> + if (err)
>> + goto err_classifier;
>> +
>> + err = insert_rule(ff, eth_rule, c->id, key, key_size);
>> + if (err) {
>> + /* destroy_classifier will free the classifier */
>> + destroy_classifier(ff, c->id);
>> + goto err_key;
>> + }
>> +
>> + return 0;
>> +
>> +err_classifier:
>> + kfree(c);
>> +err_key:
>> + kfree(key);
>> +
>> + return err;
>> +}
>> +
>> +static int virtnet_ethtool_flow_insert(struct virtnet_ff *ff,
>> + struct ethtool_rx_flow_spec *fs,
>> + u16 curr_queue_pairs)
>> +{
>> + struct virtnet_ethtool_rule *eth_rule;
>> + int err;
>> +
>> + if (!ff->ff_supported)
>> + return -EOPNOTSUPP;
>> +
>> + err = validate_flow_input(ff, fs, curr_queue_pairs);
>> + if (err)
>> + return err;
>> +
>> + eth_rule = kzalloc(sizeof(*eth_rule), GFP_KERNEL);
>> + if (!eth_rule)
>> + return -ENOMEM;
>> +
>> + err = xa_alloc(&ff->ethtool.rules, &fs->location, eth_rule,
>> + XA_LIMIT(0, le32_to_cpu(ff->ff_caps->rules_limit) - 1),
>> + GFP_KERNEL);
>> + if (err)
>> + goto err_rule;
>> +
>> + eth_rule->flow_spec = *fs;
>> +
>> + err = build_and_insert(ff, eth_rule);
>> + if (err)
>> + goto err_xa;
>> +
>> + return err;
>> +
>> +err_xa:
>> + xa_erase(&ff->ethtool.rules, eth_rule->flow_spec.location);
>> +
>> +err_rule:
>> + fs->location = RX_CLS_LOC_ANY;
>> + kfree(eth_rule);
>> +
>> + return err;
>> +}
>> +
>> +static int virtnet_ethtool_flow_remove(struct virtnet_ff *ff, int location)
>> +{
>> + struct virtnet_ethtool_rule *eth_rule;
>> + int err = 0;
>> +
>> + if (!ff->ff_supported)
>> + return -EOPNOTSUPP;
>> +
>> + eth_rule = xa_load(&ff->ethtool.rules, location);
>> + if (!eth_rule) {
>> + err = -ENOENT;
>> + goto out;
>> + }
>> +
>> + destroy_ethtool_rule(ff, eth_rule);
>> +out:
>> + return err;
>> +}
>> +
>> static size_t get_mask_size(u16 type)
>> {
>> switch (type) {
>> @@ -5875,6 +6328,8 @@ static int virtnet_ff_init(struct virtnet_ff *ff, struct virtio_device *vdev)
>> if (err)
>> goto err_ff_action;
>>
>> + xa_init_flags(&ff->classifiers, XA_FLAGS_ALLOC);
>> + xa_init_flags(&ff->ethtool.rules, XA_FLAGS_ALLOC);
>> ff->vdev = vdev;
>> ff->ff_supported = true;
>>
>> @@ -5899,9 +6354,18 @@ static int virtnet_ff_init(struct virtnet_ff *ff, struct virtio_device *vdev)
>>
>> static void virtnet_ff_cleanup(struct virtnet_ff *ff)
>> {
>> + struct virtnet_ethtool_rule *eth_rule;
>> + unsigned long i;
>> +
>> if (!ff->ff_supported)
>> return;
>>
>> + xa_for_each(&ff->ethtool.rules, i, eth_rule)
>> + destroy_ethtool_rule(ff, eth_rule);
>> +
>> + xa_destroy(&ff->ethtool.rules);
>> + xa_destroy(&ff->classifiers);
>> +
>> virtio_admin_obj_destroy(ff->vdev,
>> VIRTIO_NET_RESOURCE_OBJ_FF_GROUP,
>> VIRTNET_FF_ETHTOOL_GROUP_PRIORITY,
>> diff --git a/include/uapi/linux/virtio_net_ff.h b/include/uapi/linux/virtio_net_ff.h
>> index c7d01e3edc80..aa0619f9c63b 100644
>> --- a/include/uapi/linux/virtio_net_ff.h
>> +++ b/include/uapi/linux/virtio_net_ff.h
>> @@ -13,6 +13,8 @@
>> #define VIRTIO_NET_FF_ACTION_CAP 0x802
>>
>> #define VIRTIO_NET_RESOURCE_OBJ_FF_GROUP 0x0200
>> +#define VIRTIO_NET_RESOURCE_OBJ_FF_CLASSIFIER 0x0201
>> +#define VIRTIO_NET_RESOURCE_OBJ_FF_RULE 0x0202
>>
>> /**
>> * struct virtio_net_ff_cap_data - Flow filter resource capability limits
>> @@ -103,4 +105,52 @@ struct virtio_net_resource_obj_ff_group {
>> __le16 group_priority;
>> };
>>
>> +/**
>> + * struct virtio_net_resource_obj_ff_classifier - Flow filter classifier object
>> + * @count: number of selector entries in @selectors
>> + * @reserved: must be set to 0 by the driver and ignored by the device
>> + * @selectors: array of selector descriptors that define match masks
>> + *
>> + * Payload for the VIRTIO_NET_RESOURCE_OBJ_FF_CLASSIFIER administrative object.
>> + * Each selector describes a header mask used to match packets
>> + * (see struct virtio_net_ff_selector). Selectors appear in the order they are
>> + * to be applied.
>> + */
>> +struct virtio_net_resource_obj_ff_classifier {
>> + __u8 count;
>> + __u8 reserved[7];
>> + __u8 selectors[];
>> +};
>> +
>> +/**
>> + * struct virtio_net_resource_obj_ff_rule - Flow filter rule object
>> + * @group_id: identifier of the target flow filter group
>> + * @classifier_id: identifier of the classifier referenced by this rule
>> + * @rule_priority: relative priority of this rule within the group
>> + * @key_length: number of bytes in @keys
>> + * @action: action to perform, one of VIRTIO_NET_FF_ACTION_*
>> + * @reserved: must be set to 0 by the driver and ignored by the device
>> + * @vq_index: RX virtqueue index for VIRTIO_NET_FF_ACTION_RX_VQ, 0 otherwise
>> + * @reserved1: must be set to 0 by the driver and ignored by the device
>> + * @keys: concatenated key bytes matching the classifier's selectors order
>> + *
>> + * Payload for the VIRTIO_NET_RESOURCE_OBJ_FF_RULE administrative object.
>> + * @group_id and @classifier_id refer to previously created objects of types
>> + * VIRTIO_NET_RESOURCE_OBJ_FF_GROUP and VIRTIO_NET_RESOURCE_OBJ_FF_CLASSIFIER
>> + * respectively. The key bytes are compared against packet headers using the
>> + * masks provided by the classifier's selectors. Multi-byte fields are
>> + * little-endian.
>> + */
>> +struct virtio_net_resource_obj_ff_rule {
>> + __le32 group_id;
>> + __le32 classifier_id;
>> + __u8 rule_priority;
>> + __u8 key_length; /* length of key in bytes */
>> + __u8 action;
>> + __u8 reserved;
>> + __le16 vq_index;
>> + __u8 reserved1[2];
>> + __u8 keys[];
>> +};
>> +
>> #endif
>> --
>> 2.50.1
>
>
^ permalink raw reply
* Re: [PATCH net-next v20 05/12] virtio_net: Query and set flow filter caps
From: Shahar Shitrit @ 2026-07-09 13:37 UTC (permalink / raw)
To: Michael S. Tsirkin, Daniel Jurgens
Cc: netdev, jasowang, pabeni, virtualization, parav, yohadt, xuanzhuo,
eperezma, jgg, kevin.tian, kuba, andrew+netdev, edumazet
In-Reply-To: <20260208063807-mutt-send-email-mst@kernel.org>
On 08/02/2026 13:51, Michael S. Tsirkin wrote:
> On Thu, Feb 05, 2026 at 04:47:00PM -0600, Daniel Jurgens wrote:
>> When probing a virtnet device, attempt to read the flow filter
>> capabilities. In order to use the feature the caps must also
>> be set. For now setting what was read is sufficient.
>>
>> This patch adds uapi definitions virtio_net flow filters define in
>> version 1.4 of the VirtIO spec.
>>
>> Signed-off-by: Daniel Jurgens <danielj@nvidia.com>
>> Reviewed-by: Parav Pandit <parav@nvidia.com>
>> Reviewed-by: Shahar Shitrit <shshitrit@nvidia.com>
>>
>> ---
>> v4:
>> - Validate the length in the selector caps
>> - Removed __free usage.
>> - Removed for(int.
>> v5:
>> - Remove unneed () after MAX_SEL_LEN macro (test bot)
>> v6:
>> - Fix sparse warning "array of flexible structures" Jakub K/Simon H
>> - Use new variable and validate ff_mask_size before set_cap. MST
>> v7:
>> - Set ff->ff_{caps, mask, actions} NULL in error path. Paolo Abeni
>> - Return errors from virtnet_ff_init, -ENOTSUPP is not fatal. Xuan
>>
>> v8:
>> - Use real_ff_mask_size when setting the selector caps. Jason Wang
>>
>> v9:
>> - Set err after failed memory allocations. Simon Horman
>>
>> v10:
>> - Return -EOPNOTSUPP in virnet_ff_init before allocing any memory.
>> Jason/Paolo.
>>
>> v11:
>> - Return -EINVAL if any resource limit is 0. Simon Horman
>> - Ensure we don't overrun alloced space of ff->ff_mask by moving the
>> real_ff_mask_size > ff_mask_size check into the loop. Simon Horman
>>
>> v12:
>> - Move uapi includes to virtio_net.c vs header file. MST
>> - Remove kernel.h header in virtio_net_ff uapi. MST
>> - WARN_ON_ONCE in error paths validating selectors. MST
>> - Move includes from .h to .c files. MST
>> - Add WARN_ON_ONCE if obj_destroy fails. MST
>> - Comment cleanup in virito_net_ff.h uapi. MST
>> - Add 2 byte pad to the end of virtio_net_ff_cap_data.
>> https://lore.kernel.org/virtio-comment/20251119044029-mutt-send-email-mst@kernel.org/T/#m930988a5d3db316c68546d8b61f4b94f6ebda030
>> - Cleanup and reinit in the freeze/restore path. MST
>>
>> v13:
>> - Added /* private: */ comment before reserved field. Jakub
>> - Change ff_mask validation to break at unkonwn selector type. This
>> will allow compatability with newer controllers if the types of
>> selectors is expanded. MST
>>
>> v14:
>> - Handle err from virtnet_ff_init in virtnet_restore_up. MST
>>
>> v15:
>> - In virtnet_restore_up only call virtnet_close in err path if
>> netif_runnig. AI
>>
>> v16:
>> - Return 0 from virtnet_restore_up if virtnet_init_ff return not
>> supported. AI
>>
>> v17:
>> - During restore freeze_down on error during ff_init. AI
>>
>> v18:
>> - Changed selector cap validation to verify size for each type
>> instead of just checking they weren't bigger than max size. AI
>> - Added __count_by attribute to flexible members in uapi. Paolo A
>>
>> v19:
>> - Fixed ;; and incorrect plural in comment. AI
>>
>> v20:
>> - include uapi/linux/stddef.h for __counted_by. AI
>
> AI has led you astray, sadly (
will fix in v21
>
>
>
>
>> ---
>> drivers/net/virtio_net.c | 231 ++++++++++++++++++++++++++++-
>> include/uapi/linux/virtio_net_ff.h | 91 ++++++++++++
>> 2 files changed, 321 insertions(+), 1 deletion(-)
>> create mode 100644 include/uapi/linux/virtio_net_ff.h
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index db88dcaefb20..2cfa37e2f83f 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -26,6 +26,11 @@
>> #include <net/netdev_rx_queue.h>
>> #include <net/netdev_queues.h>
>> #include <net/xdp_sock_drv.h>
>> +#include <linux/virtio_admin.h>
>> +#include <net/ipv6.h>
>> +#include <net/ip.h>
>> +#include <uapi/linux/virtio_pci.h>
>> +#include <uapi/linux/virtio_net_ff.h>
>>
>> static int napi_weight = NAPI_POLL_WEIGHT;
>> module_param(napi_weight, int, 0444);
>> @@ -281,6 +286,14 @@ static const struct virtnet_stat_desc virtnet_stats_tx_speed_desc_qstat[] = {
>> VIRTNET_STATS_DESC_TX_QSTAT(speed, ratelimit_packets, hw_drop_ratelimits),
>> };
>>
>> +struct virtnet_ff {
>> + struct virtio_device *vdev;
>> + bool ff_supported;
>> + struct virtio_net_ff_cap_data *ff_caps;
>> + struct virtio_net_ff_cap_mask_data *ff_mask;
>> + struct virtio_net_ff_actions *ff_actions;
>> +};
>> +
>> #define VIRTNET_Q_TYPE_RX 0
>> #define VIRTNET_Q_TYPE_TX 1
>> #define VIRTNET_Q_TYPE_CQ 2
>> @@ -488,6 +501,7 @@ struct virtnet_info {
>> TRAILING_OVERLAP(struct virtio_net_rss_config_trailer, rss_trailer, hash_key_data,
>> u8 rss_hash_key_data[VIRTIO_NET_RSS_MAX_KEY_SIZE];
>> );
>> + struct virtnet_ff ff;
>> };
>> static_assert(offsetof(struct virtnet_info, rss_trailer.hash_key_data) ==
>> offsetof(struct virtnet_info, rss_hash_key_data));
>> @@ -526,6 +540,7 @@ static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
>> struct page *page, void *buf,
>> int len, int truesize);
>> static void virtnet_xsk_completed(struct send_queue *sq, int num);
>> +static void remove_vq_common(struct virtnet_info *vi);
>>
>> enum virtnet_xmit_type {
>> VIRTNET_XMIT_TYPE_SKB,
>> @@ -5684,6 +5699,192 @@ static const struct netdev_stat_ops virtnet_stat_ops = {
>> .get_base_stats = virtnet_get_base_stats,
>> };
>>
>> +static size_t get_mask_size(u16 type)
>> +{
>> + switch (type) {
>> + case VIRTIO_NET_FF_MASK_TYPE_ETH:
>> + return sizeof(struct ethhdr);
>> + case VIRTIO_NET_FF_MASK_TYPE_IPV4:
>> + return sizeof(struct iphdr);
>> + case VIRTIO_NET_FF_MASK_TYPE_IPV6:
>> + return sizeof(struct ipv6hdr);
>> + case VIRTIO_NET_FF_MASK_TYPE_TCP:
>> + return sizeof(struct tcphdr);
>> + case VIRTIO_NET_FF_MASK_TYPE_UDP:
>> + return sizeof(struct udphdr);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int virtnet_ff_init(struct virtnet_ff *ff, struct virtio_device *vdev)
>> +{
>> + size_t ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data) +
>> + sizeof(struct virtio_net_ff_selector) *
>> + VIRTIO_NET_FF_MASK_TYPE_MAX;
>> + struct virtio_admin_cmd_query_cap_id_result *cap_id_list;
>> + struct virtio_net_ff_selector *sel;
>> + unsigned long sel_types = 0;
>> + size_t real_ff_mask_size;
>> + int err;
>> + int i;
>> +
>> + if (!vdev->config->admin_cmd_exec)
>> + return -EOPNOTSUPP;
>> +
>> + cap_id_list = kzalloc(sizeof(*cap_id_list), GFP_KERNEL);
>> + if (!cap_id_list)
>> + return -ENOMEM;
>> +
>> + err = virtio_admin_cap_id_list_query(vdev, cap_id_list);
>> + if (err)
>> + goto err_cap_list;
>> +
>> + if (!(VIRTIO_CAP_IN_LIST(cap_id_list,
>> + VIRTIO_NET_FF_RESOURCE_CAP) &&
>> + VIRTIO_CAP_IN_LIST(cap_id_list,
>> + VIRTIO_NET_FF_SELECTOR_CAP) &&
>> + VIRTIO_CAP_IN_LIST(cap_id_list,
>> + VIRTIO_NET_FF_ACTION_CAP))) {
>> + err = -EOPNOTSUPP;
>> + goto err_cap_list;
>> + }
>> +
>> + ff->ff_caps = kzalloc(sizeof(*ff->ff_caps), GFP_KERNEL);
>> + if (!ff->ff_caps) {
>> + err = -ENOMEM;
>> + goto err_cap_list;
>> + }
>> +
>> + err = virtio_admin_cap_get(vdev,
>> + VIRTIO_NET_FF_RESOURCE_CAP,
>> + ff->ff_caps,
>> + sizeof(*ff->ff_caps));
>> +
>> + if (err)
>> + goto err_ff;
>> +
>> + if (!ff->ff_caps->groups_limit ||
>> + !ff->ff_caps->classifiers_limit ||
>> + !ff->ff_caps->rules_limit ||
>> + !ff->ff_caps->rules_per_group_limit) {
>> + err = -EINVAL;
>> + goto err_ff;
>> + }
>> +
>> + /* VIRTIO_NET_FF_MASK_TYPE start at 1 */
>> + for (i = 1; i <= VIRTIO_NET_FF_MASK_TYPE_MAX; i++)
>> + ff_mask_size += get_mask_size(i);
>> +
>> + ff->ff_mask = kzalloc(ff_mask_size, GFP_KERNEL);
>> + if (!ff->ff_mask) {
>> + err = -ENOMEM;
>> + goto err_ff;
>> + }
>> +
>> + err = virtio_admin_cap_get(vdev,
>> + VIRTIO_NET_FF_SELECTOR_CAP,
>> + ff->ff_mask,
>> + ff_mask_size);
>
> So ff_actions is from device and ff_actions->count does not seem to be checked.
>
> If device somehow gains a larger mask down the road, can it not then overflow?
> or malicious?
see below
>
>
>> +
>> + if (err)
>> + goto err_ff_mask;
>> +
>> + ff->ff_actions = kzalloc(sizeof(*ff->ff_actions) +
>> + VIRTIO_NET_FF_ACTION_MAX,
>> + GFP_KERNEL);
>> + if (!ff->ff_actions) {
>> + err = -ENOMEM;
>> + goto err_ff_mask;
>> + }
>> +
>> + err = virtio_admin_cap_get(vdev,
>> + VIRTIO_NET_FF_ACTION_CAP,
>> + ff->ff_actions,
>> + sizeof(*ff->ff_actions) + VIRTIO_NET_FF_ACTION_MAX);
>
> So ff_actions is from device and ff_actions->count is not checked.
>
> If device gains a ton of actions down the road, can it not then overflow?
> or malicious?
it can't overflow, as it can write up to sizeof(*ff->ff_actions) +
VIRTIO_NET_FF_ACTION_MAX bytes. But it is a valid concern in case of
someone in the future iterates on count. Better to handle it now so it
won't be forgotten. Checking that ff_actions->count doesn't exceed
VIRTIO_NET_FF_ACTION_MAX can break backward compatibility. If this max
value is extended in a new spec/controller then this driver would fail
to load. Instead, I will assign ff->ff_actions->count with the minimum
of what we got and the max.
>
>> +
>> + if (err)
>> + goto err_ff_action;
>> +
>> + err = virtio_admin_cap_set(vdev,
>> + VIRTIO_NET_FF_RESOURCE_CAP,
>> + ff->ff_caps,
>> + sizeof(*ff->ff_caps));
>> + if (err)
>> + goto err_ff_action;
>> +
>> + real_ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data);
>> + sel = (void *)&ff->ff_mask->selectors;
>> +
>> + for (i = 0; i < ff->ff_mask->count; i++) {
>> + /* If the selector type is unknown it may indicate the spec
>> + * has been revised to include new types of selectors
>> + */
>> + if (sel->type > VIRTIO_NET_FF_MASK_TYPE_MAX)
>
> do you want to check sel->type 0 too?
yes, will add in v21.
>
>> + break;
>
> but count remains unchanged? should we not to reduce count here
> so device knows what driver can drive?
we should. will add it in v21.
>
>
>> +
>> + if (sel->length != get_mask_size(sel->type) ||
>> + test_and_set_bit(sel->type, &sel_types)) {
>> + WARN_ON_ONCE(true);
>> + err = -EINVAL;
>> + goto err_ff_action;
>> + }
>> + real_ff_mask_size += sizeof(struct virtio_net_ff_selector) + sel->length;
>> + if (real_ff_mask_size > ff_mask_size) {
>> + WARN_ON_ONCE(true);
>> + err = -EINVAL;
>> + goto err_ff_action;
>> + }
>> + sel = (void *)sel + sizeof(*sel) + sel->length;
>> + }
>> +
>> + err = virtio_admin_cap_set(vdev,
>> + VIRTIO_NET_FF_SELECTOR_CAP,
>> + ff->ff_mask,
>> + real_ff_mask_size);
>> + if (err)
>> + goto err_ff_action;
>> +
>> + err = virtio_admin_cap_set(vdev,
>> + VIRTIO_NET_FF_ACTION_CAP,
>> + ff->ff_actions,
>> + sizeof(*ff->ff_actions) + VIRTIO_NET_FF_ACTION_MAX);
>> + if (err)
>> + goto err_ff_action;
>> +
>> + ff->vdev = vdev;
>> + ff->ff_supported = true;
>> +
>> + kfree(cap_id_list);
>> +
>> + return 0;
>> +
>> +err_ff_action:
>> + kfree(ff->ff_actions);
>> + ff->ff_actions = NULL;
>> +err_ff_mask:
>> + kfree(ff->ff_mask);
>> + ff->ff_mask = NULL;
>> +err_ff:
>> + kfree(ff->ff_caps);
>> + ff->ff_caps = NULL;
>> +err_cap_list:
>> + kfree(cap_id_list);
>> +
>> + return err;
>> +}
>> +
>> +static void virtnet_ff_cleanup(struct virtnet_ff *ff)
>> +{
>> + if (!ff->ff_supported)
>> + return;
>> +
>> + kfree(ff->ff_actions);
>> + kfree(ff->ff_mask);
>> + kfree(ff->ff_caps);
>> + ff->ff_supported = false;
>> +}
>> +
>> static void virtnet_freeze_down(struct virtio_device *vdev)
>> {
>> struct virtnet_info *vi = vdev->priv;
>> @@ -5702,6 +5903,10 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
>> netif_tx_lock_bh(vi->dev);
>> netif_device_detach(vi->dev);
>> netif_tx_unlock_bh(vi->dev);
>> +
>> + rtnl_lock();
>> + virtnet_ff_cleanup(&vi->ff);
>> + rtnl_unlock();
>> }
>>
>> static int init_vqs(struct virtnet_info *vi);
>> @@ -5727,10 +5932,23 @@ static int virtnet_restore_up(struct virtio_device *vdev)
>> return err;
>> }
>>
>> + /* Initialize flow filters. Not supported is an acceptable and common
>> + * return code
>> + */
>> + rtnl_lock();
>> + err = virtnet_ff_init(&vi->ff, vi->vdev);
>> + if (err && err != -EOPNOTSUPP) {
>> + rtnl_unlock();
>> + virtnet_freeze_down(vi->vdev);
>> + remove_vq_common(vi);
>> + return err;
>> + }
>> + rtnl_unlock();
>> +
>> netif_tx_lock_bh(vi->dev);
>> netif_device_attach(vi->dev);
>> netif_tx_unlock_bh(vi->dev);
>> - return err;
>> + return 0;
>> }
>>
>> static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64 offloads)
>> @@ -7058,6 +7276,15 @@ static int virtnet_probe(struct virtio_device *vdev)
>> }
>> vi->guest_offloads_capable = vi->guest_offloads;
>>
>> + /* Initialize flow filters. Not supported is an acceptable and common
>> + * return code
>> + */
>> + err = virtnet_ff_init(&vi->ff, vi->vdev);
>> + if (err && err != -EOPNOTSUPP) {
>> + rtnl_unlock();
>> + goto free_unregister_netdev;
>> + }
>> +
>> rtnl_unlock();
>>
>> err = virtnet_cpu_notif_add(vi);
>> @@ -7073,6 +7300,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>>
>> free_unregister_netdev:
>> unregister_netdev(dev);
>> + virtnet_ff_cleanup(&vi->ff);
>> free_failover:
>> net_failover_destroy(vi->failover);
>> free_vqs:
>> @@ -7121,6 +7349,7 @@ static void virtnet_remove(struct virtio_device *vdev)
>> virtnet_free_irq_moder(vi);
>>
>> unregister_netdev(vi->dev);
>> + virtnet_ff_cleanup(&vi->ff);
>>
>> net_failover_destroy(vi->failover);
>>
>> diff --git a/include/uapi/linux/virtio_net_ff.h b/include/uapi/linux/virtio_net_ff.h
>> new file mode 100644
>> index 000000000000..552a6b3a8a91
>> --- /dev/null
>> +++ b/include/uapi/linux/virtio_net_ff.h
>> @@ -0,0 +1,91 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
>> + *
>> + * Header file for virtio_net flow filters
>> + */
>> +#ifndef _LINUX_VIRTIO_NET_FF_H
>> +#define _LINUX_VIRTIO_NET_FF_H
>> +
>> +#include <linux/types.h>
>> +#include <uapi/linux/stddef.h>
>> +
>> +#define VIRTIO_NET_FF_RESOURCE_CAP 0x800
>> +#define VIRTIO_NET_FF_SELECTOR_CAP 0x801
>> +#define VIRTIO_NET_FF_ACTION_CAP 0x802
>> +
>> +/**
>> + * struct virtio_net_ff_cap_data - Flow filter resource capability limits
>> + * @groups_limit: maximum number of flow filter groups supported by the device
>> + * @classifiers_limit: maximum number of classifiers supported by the device
>> + * @rules_limit: maximum number of rules supported device-wide across all groups
>> + * @rules_per_group_limit: maximum number of rules allowed in a single group
>> + * @last_rule_priority: priority value associated with the lowest-priority rule
>> + * @selectors_per_classifier_limit: maximum selectors allowed in one classifier
>> + */
>> +struct virtio_net_ff_cap_data {
>> + __le32 groups_limit;
>> + __le32 classifiers_limit;
>> + __le32 rules_limit;
>> + __le32 rules_per_group_limit;
>> + __u8 last_rule_priority;
>> + __u8 selectors_per_classifier_limit;
>> + /* private: */
>> + __u8 reserved[2];
>> +};
>> +
>> +/**
>> + * struct virtio_net_ff_selector - Selector mask descriptor
>> + * @type: selector type, one of VIRTIO_NET_FF_MASK_TYPE_* constants
>> + * @flags: selector flags, see VIRTIO_NET_FF_MASK_F_* constants
>> + * @reserved: must be set to 0 by the driver and ignored by the device
>> + * @length: size in bytes of @mask
>> + * @reserved1: must be set to 0 by the driver and ignored by the device
>> + * @mask: variable-length mask payload for @type, length given by @length
>> + *
>> + * A selector describes a header mask that a classifier can apply. The format
>> + * of @mask depends on @type.
>> + */
>> +struct virtio_net_ff_selector {
>> + __u8 type;
>> + __u8 flags;
>> + __u8 reserved[2];
>> + __u8 length;
>> + __u8 reserved1[3];
>> + __u8 mask[] __counted_by(length);
>> +};
>> +
>> +#define VIRTIO_NET_FF_MASK_TYPE_ETH 1
>> +#define VIRTIO_NET_FF_MASK_TYPE_IPV4 2
>> +#define VIRTIO_NET_FF_MASK_TYPE_IPV6 3
>> +#define VIRTIO_NET_FF_MASK_TYPE_TCP 4
>> +#define VIRTIO_NET_FF_MASK_TYPE_UDP 5
>> +#define VIRTIO_NET_FF_MASK_TYPE_MAX VIRTIO_NET_FF_MASK_TYPE_UDP
>> +
>> +/**
>> + * struct virtio_net_ff_cap_mask_data - Supported selector mask formats
>> + * @count: number of entries in @selectors
>> + * @reserved: must be set to 0 by the driver and ignored by the device
>> + * @selectors: packed array of struct virtio_net_ff_selector.
>> + */
>> +struct virtio_net_ff_cap_mask_data {
>> + __u8 count;
>> + __u8 reserved[7];
>> + __u8 selectors[] __counted_by(count);
>
> This looks wrong to me. count is # of selectors (packed entries) not
> bytes.
right, will remove it.
> >
>
>
>> +};
>> +
>> +#define VIRTIO_NET_FF_MASK_F_PARTIAL_MASK (1 << 0)
>> +
>> +#define VIRTIO_NET_FF_ACTION_DROP 1
>> +#define VIRTIO_NET_FF_ACTION_RX_VQ 2
>> +#define VIRTIO_NET_FF_ACTION_MAX VIRTIO_NET_FF_ACTION_RX_VQ
>> +/**
>> + * struct virtio_net_ff_actions - Supported flow actions
>> + * @count: number of supported actions in @actions
>> + * @reserved: must be set to 0 by the driver and ignored by the device
>> + * @actions: array of action identifiers (VIRTIO_NET_FF_ACTION_*)
>> + */
>> +struct virtio_net_ff_actions {
>> + __u8 count;
>> + __u8 reserved[7];
>> + __u8 actions[] __counted_by(count);
>
>
> this too.
count directly represents the number of __u8 elements in actions[], so
the size of the flexible array is count * sizeof(__u8). Therefore, here
it is correct.
>
>> +};
>> +#endif
>> --
>> 2.50.1
>
>
^ permalink raw reply
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock
From: Dmitry Osipenko @ 2026-07-09 12:55 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: <18c09efbacafbff3.b25d31827bed23c9.829612c1dfc3d1e4@ryasuoka-thinkpadx1carbongen9.tokyo.csb>
On 7/9/26 15:30, Ryosuke Yasuoka wrote:
>
>
> On 07/07/2026 18:10, Dmitry Osipenko wrote:
>> On 7/3/26 08:58, Ryosuke Yasuoka wrote:
>>>
>>>
>>> On 02/07/2026 14:26, Dmitry Osipenko wrote:
>>>> On 7/1/26 12:23, Ryosuke Yasuoka wrote:
>>>>>
>>>>>
>>>>> On 30/06/2026 16:46, Dmitry Osipenko wrote:
>>>>>> 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,
>>>>>>
>>>>>
>>>>> Hi,
>>>>> Thank you for your review.
>>>>>
>>>>>> 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.
>>>>>
>>>>> IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event()
>>>>> into virtio_gpu_init() after wait_event_timeout() would not prevent the
>>>>> issue.
>>>>>
>>>>> Looking at the syzbot call traces[1][2], the deadlock occurs during
>>>>> drm_client_setup(), which runs after virtio_gpu_init() has already
>>>>> returned. The display_info_cb that triggers the deadlock is called from
>>>>> the dequeue worker while drm_client_register() holds clientlist_mutex.
>>>>>
>>>>> Thread A:
>>>>> virtio_gpu_probe()
>>>>> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s
>>>>
>>>> You mean that the timeout happens and it's again syzkaller report for a
>>>> broken host. A day ago I applied [1] that should fix this "bogus"
>>>> syzkaller report.
>>>>
>>>> [1] https://patchwork.freedesktop.org/patch/735301/
>>>
>>> Thank you for [1] and the clarificaiton. I agree that [1] addresses the
>>> broken host scenario where virtio_gpu_init() times out and probe
>>> continues with a pending display_info response. I think there is another
>>> scenario causing the same deadlock reported by syzbot and my patch
>>> addresses both cases.
>>>
>>> The deadlock can also occur with the following scenario:
>>> after virtio_gpu_init() completes successfully (wait_event_timeout
>>> returns), if a config_changed event arrives before drm_client_setup()
>>> completes, config_changed_work_func() sends a new GET_DISPLAY_INFO. Its
>>> display_info_cb fires in the dequeue worker and blocks on
>>> clientlist_mutex held by drm_client_register(). This is the same
>>> deadlock path reported by syzbot.
>>>
>>> Since [1] only prevents the timeout case, I believe the schedule_work()
>>> approach is still needed to prevent display_info_cb from blocking on
>>> clientlist_mutex in the dequeue worker context, regardless of the
>>> trigger.
>>
>> The config_changed_work_func() already invokes
>> drm_helper_hpd_irq_event() by itself [1].
>>
>> [1]
>> https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/virtio/virtgpu_kms.c#L52
>>
>> The drm_helper_hpd_irq_event() itself calls
>> drm_kms_helper_hotplug_event() [2].
>>
>> [2]
>> https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/drm_probe_helper.c#L1113
>>
>> Isn't drm_helper_hpd_irq_event() call done by
>> virtio_gpu_cmd_get_display_info_cb() already partially redundant? I.e.
>> only it's needed by virtio_gpu_init() and not by
>> config_changed_work_func(). And then we can move
>> drm_helper_hpd_irq_event() to virtio_gpu_init():
>>
>> ```
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
>> b/drivers/gpu/drm/virtio/virtgpu_kms.c
>> index 922782bdc9cd..9805283683c1 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
>> @@ -303,6 +303,7 @@ int virtio_gpu_init(struct virtio_device *vdev,
>> struct drm_device *dev)
>> virtio_gpu_notify(vgdev);
>> wait_event_timeout(vgdev->resp_wq,
>> !vgdev->display_info_pending,
>> 5 * HZ);
>> + drm_helper_hpd_irq_event(vgdev->ddev);
>> }
>>
>> vgdev->pm_nb.notifier_call = virtio_gpu_pm_notifier;
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
>> b/drivers/gpu/drm/virtio/virtgpu_vq.c
>> index 68d097ad9d1d..aa6440a9b81e 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
>> @@ -880,9 +880,6 @@ static void
>> virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
>> vgdev->display_info_pending = false;
>> 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);
>> }
>>
>> static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device
>> *vgdev,
>> ```
>>
>> Isn't it a bug that config_changed_work_func() doesn't wait for
>> display_info_pending event?
>>
>>
>> ```
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
>> b/drivers/gpu/drm/virtio/virtgpu_kms.c
>> index 922782bdc9cd..dd2d5c896c39 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
>> @@ -50,6 +50,8 @@ static void virtio_gpu_config_changed_work_func(struct
>> work_struct *work)
>> 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);
>> drm_helper_hpd_irq_event(vgdev->ddev);
>> }
>> events_clear |= VIRTIO_GPU_EVENT_DISPLAY;
>>
>> ```
>
> Thank you for your suggestion. I agree that removing the hotplug calls
> from display_info_cb and waiting for the response in
> config_changed_work_func() and virtio_gpu_init() is a cleaner approach.
> I'll send a v2 based on this.
Thanks, please also check whether drm_helper_hpd_irq_event() needed at
all in in virtio_gpu_init(). Suppose the hotplug event does nothing
before DRM device/client has been registered.
--
Best regards,
Dmitry
^ permalink raw reply
* [PATCH v2] virtio-pmem: allocate flush bio from a driver-private bio_set
From: Joseph Qi @ 2026-07-09 12:44 UTC (permalink / raw)
To: Pankaj Gupta; +Cc: Christoph Hellwig, Baokun Li, virtualization, linux-kernel
async_pmem_flush() allocates a child bio for the flush with GFP_ATOMIC.
This runs from pmem_submit_bio(), a ->submit_bio callback that executes
in a sleepable context, so there is no atomicity requirement here.
bio_alloc() only guarantees success when __GFP_DIRECT_RECLAIM is set,
because that is what lets it fall back to the mempool reserve. With
GFP_ATOMIC the reclaim bit is absent, so the allocation can fail and
return -ENOMEM whenever the fast paths (percpu cache and slab) are
exhausted, which is common right after boot. A flush is issued from
filesystem writeback and must not fail on a transient allocation
shortage, otherwise the device can appear unmountable:
Buffer I/O error on dev pmem0, logical block 0, lost sync page write
Switch to GFP_NOIO so __GFP_DIRECT_RECLAIM is set and the allocation can
make forward progress. However, bio_alloc() draws from the shared
fs_bio_set, and the incoming bio being flushed may itself have come from
fs_bio_set; allocating a second bio from the same set while submitting
underneath ->submit_bio can deadlock the mempool. Add a driver-private
bio_set for the flush and allocate from it via bio_alloc_bioset(), so
the flush bio has an independent reserve.
With a dedicated mempool-backed bio_set and GFP_NOIO the allocation
cannot fail, so drop the now-redundant NULL check.
Fixes: 6e84200c0a29 ("virtio-pmem: Add virtio pmem driver")
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
drivers/nvdimm/nd_virtio.c | 11 ++++++-----
drivers/nvdimm/virtio_pmem.c | 11 ++++++++++-
drivers/nvdimm/virtio_pmem.h | 4 ++++
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index 4176046627beb..b4bd21edf5c1c 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -110,17 +110,18 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
/* The asynchronous flush callback function */
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;
+
/*
* 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 bio *child = bio_alloc_bioset(bio->bi_bdev, 0,
+ REQ_OP_WRITE | REQ_PREFLUSH, GFP_NOIO,
+ &vpmem->flush_bio_set);
- if (!child)
- return -ENOMEM;
bio_clone_blkg_association(child, bio);
child->bi_iter.bi_sector = -1;
bio_chain(child, bio);
diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
index 77b1966619059..136179506b478 100644
--- a/drivers/nvdimm/virtio_pmem.c
+++ b/drivers/nvdimm/virtio_pmem.c
@@ -65,12 +65,17 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
}
mutex_init(&vpmem->flush_lock);
+ err = bioset_init(&vpmem->flush_bio_set, BIO_POOL_SIZE, 0, 0);
+ if (err) {
+ dev_err(&vdev->dev, "failed to initialize flush bio_set\n");
+ goto out_err;
+ }
vpmem->vdev = vdev;
vdev->priv = vpmem;
err = init_vq(vpmem);
if (err) {
dev_err(&vdev->dev, "failed to initialize virtio pmem vq's\n");
- goto out_err;
+ goto out_bioset;
}
if (virtio_has_feature(vdev, VIRTIO_PMEM_F_SHMEM_REGION)) {
@@ -131,6 +136,8 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
nvdimm_bus_unregister(vpmem->nvdimm_bus);
out_vq:
vdev->config->del_vqs(vdev);
+out_bioset:
+ bioset_exit(&vpmem->flush_bio_set);
out_err:
return err;
}
@@ -138,10 +145,12 @@ 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);
vdev->config->del_vqs(vdev);
virtio_reset_device(vdev);
+ bioset_exit(&vpmem->flush_bio_set);
}
static int virtio_pmem_freeze(struct virtio_device *vdev)
diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
index f72cf17f9518f..4ff2076f75047 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/bio.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;
+ /* bio_set for allocating flush child bios */
+ struct bio_set flush_bio_set;
+
/* nvdimm bus registers virtio pmem device */
struct nvdimm_bus *nvdimm_bus;
struct nvdimm_bus_descriptor nd_desc;
--
2.39.3
^ permalink raw reply related
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock
From: Ryosuke Yasuoka @ 2026-07-09 12:30 UTC (permalink / raw)
To: Dmitry Osipenko, 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: <8b02c456-fb23-4c17-ad20-a2bab2d588b1@collabora.com>
On 07/07/2026 18:10, Dmitry Osipenko wrote:
> On 7/3/26 08:58, Ryosuke Yasuoka wrote:
>>
>>
>> On 02/07/2026 14:26, Dmitry Osipenko wrote:
>>> On 7/1/26 12:23, Ryosuke Yasuoka wrote:
>>>>
>>>>
>>>> On 30/06/2026 16:46, Dmitry Osipenko wrote:
>>>>> 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,
>>>>>
>>>>
>>>> Hi,
>>>> Thank you for your review.
>>>>
>>>>> 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.
>>>>
>>>> IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event()
>>>> into virtio_gpu_init() after wait_event_timeout() would not prevent the
>>>> issue.
>>>>
>>>> Looking at the syzbot call traces[1][2], the deadlock occurs during
>>>> drm_client_setup(), which runs after virtio_gpu_init() has already
>>>> returned. The display_info_cb that triggers the deadlock is called from
>>>> the dequeue worker while drm_client_register() holds clientlist_mutex.
>>>>
>>>> Thread A:
>>>> virtio_gpu_probe()
>>>> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s
>>>
>>> You mean that the timeout happens and it's again syzkaller report for a
>>> broken host. A day ago I applied [1] that should fix this "bogus"
>>> syzkaller report.
>>>
>>> [1] https://patchwork.freedesktop.org/patch/735301/
>>
>> Thank you for [1] and the clarificaiton. I agree that [1] addresses the
>> broken host scenario where virtio_gpu_init() times out and probe
>> continues with a pending display_info response. I think there is another
>> scenario causing the same deadlock reported by syzbot and my patch
>> addresses both cases.
>>
>> The deadlock can also occur with the following scenario:
>> after virtio_gpu_init() completes successfully (wait_event_timeout
>> returns), if a config_changed event arrives before drm_client_setup()
>> completes, config_changed_work_func() sends a new GET_DISPLAY_INFO. Its
>> display_info_cb fires in the dequeue worker and blocks on
>> clientlist_mutex held by drm_client_register(). This is the same
>> deadlock path reported by syzbot.
>>
>> Since [1] only prevents the timeout case, I believe the schedule_work()
>> approach is still needed to prevent display_info_cb from blocking on
>> clientlist_mutex in the dequeue worker context, regardless of the
>> trigger.
>
> The config_changed_work_func() already invokes
> drm_helper_hpd_irq_event() by itself [1].
>
> [1]
> https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/virtio/virtgpu_kms.c#L52
>
> The drm_helper_hpd_irq_event() itself calls
> drm_kms_helper_hotplug_event() [2].
>
> [2]
> https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/drm_probe_helper.c#L1113
>
> Isn't drm_helper_hpd_irq_event() call done by
> virtio_gpu_cmd_get_display_info_cb() already partially redundant? I.e.
> only it's needed by virtio_gpu_init() and not by
> config_changed_work_func(). And then we can move
> drm_helper_hpd_irq_event() to virtio_gpu_init():
>
> ```
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
> b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index 922782bdc9cd..9805283683c1 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -303,6 +303,7 @@ int virtio_gpu_init(struct virtio_device *vdev,
> struct drm_device *dev)
> virtio_gpu_notify(vgdev);
> wait_event_timeout(vgdev->resp_wq,
> !vgdev->display_info_pending,
> 5 * HZ);
> + drm_helper_hpd_irq_event(vgdev->ddev);
> }
>
> vgdev->pm_nb.notifier_call = virtio_gpu_pm_notifier;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
> b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 68d097ad9d1d..aa6440a9b81e 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -880,9 +880,6 @@ static void
> virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
> vgdev->display_info_pending = false;
> 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);
> }
>
> static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device
> *vgdev,
> ```
>
> Isn't it a bug that config_changed_work_func() doesn't wait for
> display_info_pending event?
>
>
> ```
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
> b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index 922782bdc9cd..dd2d5c896c39 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -50,6 +50,8 @@ static void virtio_gpu_config_changed_work_func(struct
> work_struct *work)
> 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);
> drm_helper_hpd_irq_event(vgdev->ddev);
> }
> events_clear |= VIRTIO_GPU_EVENT_DISPLAY;
>
> ```
Thank you for your suggestion. I agree that removing the hotplug calls
from display_info_cb and waiting for the response in
config_changed_work_func() and virtio_gpu_init() is a cleaner approach.
I'll send a v2 based on this.
Best regards,
Ryosuke
^ permalink raw reply
* [PATCH 36/60] drm/virtio: Convert to atomic_create_state
From: Maxime Ripard @ 2026-07-09 11:51 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, Maxime Ripard, airlied, dmitry.osipenko,
gurchetansingh, kraxel, olvaffe, virtualization
In-Reply-To: <20260709-drm-no-more-plane-reset-v1-0-302d986fe5f0@kernel.org>
The plane only initializes a pristine state in its reset hook
using drm_atomic_helper_plane_reset(), which is equivalent to what
atomic_create_state expects. Convert to it.
The conversion was done using the following Coccinelle semantic patch:
@@
identifier funcs;
symbol drm_atomic_helper_plane_reset;
symbol drm_atomic_helper_plane_create_state;
@@
struct drm_plane_funcs funcs = {
...,
- .reset = drm_atomic_helper_plane_reset,
+ .atomic_create_state = drm_atomic_helper_plane_create_state,
...,
};
@match_struct_reset@
identifier funcs, reset_func;
@@
struct drm_plane_funcs funcs = {
...,
.reset = reset_func,
...,
};
@reset_uses_helpers depends on match_struct_reset@
identifier match_struct_reset.reset_func;
@@
void reset_func(...)
{
<+...
(
__drm_atomic_helper_plane_reset(...);
|
__drm_gem_reset_shadow_plane(...);
)
...+>
}
@match_struct_destroy@
identifier funcs, destroy_func;
@@
struct drm_plane_funcs funcs = {
...,
.atomic_destroy_state = destroy_func,
...,
};
@script:python renamed_func@
old_name << match_struct_reset.reset_func;
new_name;
@@
if old_name.endswith("_reset"):
coccinelle.new_name = old_name.replace("_reset", "_create_state")
else:
coccinelle.new_name = old_name
@update_struct depends on match_struct_reset && reset_uses_helpers@
identifier match_struct_reset.funcs, match_struct_reset.reset_func;
identifier renamed_func.new_name;
@@
struct drm_plane_funcs funcs = {
...,
- .reset = reset_func,
+ .atomic_create_state = new_name,
...,
};
@drop_destroy depends on update_struct && match_struct_destroy@
identifier match_struct_reset.reset_func;
identifier match_struct_destroy.destroy_func;
identifier container_func;
identifier P;
symbol drm_atomic_helper_plane_destroy_state;
symbol __drm_atomic_helper_plane_destroy_state;
@@
void reset_func(struct drm_plane *P)
{
...
(
- if (P->state) {
- <+...
(
- drm_atomic_helper_plane_destroy_state(P, P->state);
|
- __drm_atomic_helper_plane_destroy_state(P->state);
|
- P->funcs->atomic_destroy_state(P, P->state);
|
- destroy_func(P, P->state);
)
- ...+>
- }
|
- drm_WARN_ON_ONCE(P->dev, P->state);
|
- WARN_ON(P->state);
)
...
(
- kfree(P->state);
|
- kfree(container_func(P->state));
|
// kfree is optional
)
(
- P->state = NULL;
|
// plane->state clearing is optional
)
...
}
@drop_destroy_mtk depends on update_struct@
identifier P;
symbol __drm_atomic_helper_plane_destroy_state;
symbol to_mtk_plane_state;
@@
void mtk_plane_reset(struct drm_plane *P)
{
...
- if (P->state) {
- __drm_atomic_helper_plane_destroy_state(P->state);
- ...
- } else {
...
- }
...
}
@transform_nv50_wndw depends on update_struct@
identifier S;
@@
void nv50_wndw_reset(...)
{
...
- if (WARN_ON(!(S = kzalloc_obj(*S))))
+ S = kzalloc_obj(*S);
+ if (WARN_ON(!S))
return;
...
}
@transform_kzalloc depends on update_struct@
identifier match_struct_reset.reset_func;
identifier P, S;
statement ST;
statement list STL;
@@
void reset_func(struct drm_plane *P)
{
<...
S = kzalloc_obj(*S);
(
- if (S)
- {
- STL
- }
+ if (!S) return;
+
+ STL
|
- if (S) ST
+ if (!S) return;
+
+ ST
)
...>
}
@transform_body depends on update_struct@
identifier match_struct_reset.reset_func;
identifier renamed_func.new_name;
identifier S, P;
expression PS;
@@
- void reset_func(struct drm_plane *P)
+ struct drm_plane_state *new_name(struct drm_plane *P)
{
...
S = kzalloc_obj(*S);
...
(
if (!S) {
...
- return;
+ return ERR_PTR(-ENOMEM);
}
|
if (WARN_ON(!S)) {
...
- return;
+ return ERR_PTR(-ENOMEM);
}
|
if (S == NULL) {
...
- return;
+ return ERR_PTR(-ENOMEM);
}
)
...
(
- __drm_atomic_helper_plane_reset(P, PS);
+ __drm_atomic_helper_plane_state_init(PS, P);
|
- __drm_gem_reset_shadow_plane(P, PS);
+ __drm_gem_shadow_plane_state_init(P, PS);
)
...
}
@update_early_return depends on update_struct@
identifier match_struct_reset.reset_func;
identifier renamed_func.new_name;
identifier P;
expression PS;
@@
struct drm_plane_state *new_name(struct drm_plane *P)
{
<+...
- return;
+ return ERR_PTR(-EINVAL);
...+>
}
@update_return_plane depends on update_struct@
identifier match_struct_reset.reset_func;
identifier renamed_func.new_name;
identifier P;
expression PS;
@@
struct drm_plane_state *new_name(struct drm_plane *P)
{
...
__drm_atomic_helper_plane_state_init(PS, P);
...
+
+ return PS;
}
@update_return_shadow depends on update_struct@
identifier renamed_func.new_name;
identifier P;
expression PS;
@@
struct drm_plane_state *new_name(struct drm_plane *P)
{
...
__drm_gem_shadow_plane_state_init(P, PS);
...
+
+ return &PS->base;
}
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: airlied@redhat.com
Cc: dmitry.osipenko@collabora.com
Cc: gurchetansingh@chromium.org
Cc: kraxel@redhat.com
Cc: olvaffe@gmail.com
Cc: virtualization@lists.linux.dev
---
drivers/gpu/drm/virtio/virtgpu_plane.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 1d1b27ece62a..52971864db50 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -89,11 +89,11 @@ drm_plane_state *virtio_gpu_plane_duplicate_state(struct drm_plane *plane)
}
static const struct drm_plane_funcs virtio_gpu_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
- .reset = drm_atomic_helper_plane_reset,
+ .atomic_create_state = drm_atomic_helper_plane_create_state,
.atomic_duplicate_state = virtio_gpu_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
};
static int virtio_gpu_plane_atomic_check(struct drm_plane *plane,
--
2.54.0
^ permalink raw reply related
* [PATCH 27/60] drm/qxl: Convert to atomic_create_state
From: Maxime Ripard @ 2026-07-09 11:50 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, Maxime Ripard, airlied, kraxel, spice-devel,
virtualization
In-Reply-To: <20260709-drm-no-more-plane-reset-v1-0-302d986fe5f0@kernel.org>
The plane only initializes a pristine state in its reset hook
using drm_atomic_helper_plane_reset(), which is equivalent to what
atomic_create_state expects. Convert to it.
The conversion was done using the following Coccinelle semantic patch:
@@
identifier funcs;
symbol drm_atomic_helper_plane_reset;
symbol drm_atomic_helper_plane_create_state;
@@
struct drm_plane_funcs funcs = {
...,
- .reset = drm_atomic_helper_plane_reset,
+ .atomic_create_state = drm_atomic_helper_plane_create_state,
...,
};
@match_struct_reset@
identifier funcs, reset_func;
@@
struct drm_plane_funcs funcs = {
...,
.reset = reset_func,
...,
};
@reset_uses_helpers depends on match_struct_reset@
identifier match_struct_reset.reset_func;
@@
void reset_func(...)
{
<+...
(
__drm_atomic_helper_plane_reset(...);
|
__drm_gem_reset_shadow_plane(...);
)
...+>
}
@match_struct_destroy@
identifier funcs, destroy_func;
@@
struct drm_plane_funcs funcs = {
...,
.atomic_destroy_state = destroy_func,
...,
};
@script:python renamed_func@
old_name << match_struct_reset.reset_func;
new_name;
@@
if old_name.endswith("_reset"):
coccinelle.new_name = old_name.replace("_reset", "_create_state")
else:
coccinelle.new_name = old_name
@update_struct depends on match_struct_reset && reset_uses_helpers@
identifier match_struct_reset.funcs, match_struct_reset.reset_func;
identifier renamed_func.new_name;
@@
struct drm_plane_funcs funcs = {
...,
- .reset = reset_func,
+ .atomic_create_state = new_name,
...,
};
@drop_destroy depends on update_struct && match_struct_destroy@
identifier match_struct_reset.reset_func;
identifier match_struct_destroy.destroy_func;
identifier container_func;
identifier P;
symbol drm_atomic_helper_plane_destroy_state;
symbol __drm_atomic_helper_plane_destroy_state;
@@
void reset_func(struct drm_plane *P)
{
...
(
- if (P->state) {
- <+...
(
- drm_atomic_helper_plane_destroy_state(P, P->state);
|
- __drm_atomic_helper_plane_destroy_state(P->state);
|
- P->funcs->atomic_destroy_state(P, P->state);
|
- destroy_func(P, P->state);
)
- ...+>
- }
|
- drm_WARN_ON_ONCE(P->dev, P->state);
|
- WARN_ON(P->state);
)
...
(
- kfree(P->state);
|
- kfree(container_func(P->state));
|
// kfree is optional
)
(
- P->state = NULL;
|
// plane->state clearing is optional
)
...
}
@drop_destroy_mtk depends on update_struct@
identifier P;
symbol __drm_atomic_helper_plane_destroy_state;
symbol to_mtk_plane_state;
@@
void mtk_plane_reset(struct drm_plane *P)
{
...
- if (P->state) {
- __drm_atomic_helper_plane_destroy_state(P->state);
- ...
- } else {
...
- }
...
}
@transform_nv50_wndw depends on update_struct@
identifier S;
@@
void nv50_wndw_reset(...)
{
...
- if (WARN_ON(!(S = kzalloc_obj(*S))))
+ S = kzalloc_obj(*S);
+ if (WARN_ON(!S))
return;
...
}
@transform_kzalloc depends on update_struct@
identifier match_struct_reset.reset_func;
identifier P, S;
statement ST;
statement list STL;
@@
void reset_func(struct drm_plane *P)
{
<...
S = kzalloc_obj(*S);
(
- if (S)
- {
- STL
- }
+ if (!S) return;
+
+ STL
|
- if (S) ST
+ if (!S) return;
+
+ ST
)
...>
}
@transform_body depends on update_struct@
identifier match_struct_reset.reset_func;
identifier renamed_func.new_name;
identifier S, P;
expression PS;
@@
- void reset_func(struct drm_plane *P)
+ struct drm_plane_state *new_name(struct drm_plane *P)
{
...
S = kzalloc_obj(*S);
...
(
if (!S) {
...
- return;
+ return ERR_PTR(-ENOMEM);
}
|
if (WARN_ON(!S)) {
...
- return;
+ return ERR_PTR(-ENOMEM);
}
|
if (S == NULL) {
...
- return;
+ return ERR_PTR(-ENOMEM);
}
)
...
(
- __drm_atomic_helper_plane_reset(P, PS);
+ __drm_atomic_helper_plane_state_init(PS, P);
|
- __drm_gem_reset_shadow_plane(P, PS);
+ __drm_gem_shadow_plane_state_init(P, PS);
)
...
}
@update_early_return depends on update_struct@
identifier match_struct_reset.reset_func;
identifier renamed_func.new_name;
identifier P;
expression PS;
@@
struct drm_plane_state *new_name(struct drm_plane *P)
{
<+...
- return;
+ return ERR_PTR(-EINVAL);
...+>
}
@update_return_plane depends on update_struct@
identifier match_struct_reset.reset_func;
identifier renamed_func.new_name;
identifier P;
expression PS;
@@
struct drm_plane_state *new_name(struct drm_plane *P)
{
...
__drm_atomic_helper_plane_state_init(PS, P);
...
+
+ return PS;
}
@update_return_shadow depends on update_struct@
identifier renamed_func.new_name;
identifier P;
expression PS;
@@
struct drm_plane_state *new_name(struct drm_plane *P)
{
...
__drm_gem_shadow_plane_state_init(P, PS);
...
+
+ return &PS->base;
}
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: airlied@redhat.com
Cc: kraxel@redhat.com
Cc: spice-devel@lists.freedesktop.org
Cc: virtualization@lists.linux.dev
---
drivers/gpu/drm/qxl/qxl_display.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index a026bd35ef48..d0e47be7febd 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -927,11 +927,11 @@ static const struct drm_plane_helper_funcs qxl_cursor_helper_funcs = {
static const struct drm_plane_funcs qxl_cursor_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = drm_plane_helper_destroy,
- .reset = drm_atomic_helper_plane_reset,
+ .atomic_create_state = drm_atomic_helper_plane_create_state,
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
};
static const uint32_t qxl_primary_plane_formats[] = {
@@ -949,11 +949,11 @@ static const struct drm_plane_helper_funcs primary_helper_funcs = {
static const struct drm_plane_funcs qxl_primary_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = drm_plane_helper_destroy,
- .reset = drm_atomic_helper_plane_reset,
+ .atomic_create_state = drm_atomic_helper_plane_create_state,
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
};
static struct drm_plane *qxl_create_plane(struct qxl_device *qdev,
--
2.54.0
^ permalink raw reply related
* [PATCH 00/60] drm/plane: Convert all drivers to atomic_create_state and remove reset
From: Maxime Ripard @ 2026-07-09 11:50 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, Maxime Ripard, javierm, ikerpedrosam, admin,
gargaditya08, hamohammed.sa, louis.chauvet, melissa.srw,
alexander.deucher, amd-gfx, christian.koenig, liviu.dudau,
alison.wang, stefan, jstultz, sumit.semwal, xinliang.liu,
yongqin.liu, Frank.Li, Sascha Hauer, festevam, imx, kernel,
victor.liu, l.stach, laurentiu.palcu, linux-mips, paul,
anitha.chrisanthus, paulk, chenhuacai, jeffbai, lvjianmin,
wuqianhai, xry111, zhengxingda, jbrunet, khilman, linux-amlogic,
martin.blumenstingl, abhinav.kumar, freedreno, jesszhan0024,
linux-arm-msm, lumag, marijn.suijten, robin.clark, sean, marex,
airlied, kraxel, spice-devel, virtualization, andy.yan, heiko,
hjc, linux-rockchip, Baolin Wang, orsonzhai, zhang.lyra,
alain.volmat, rgallaispou, alexandre.torgue, mcoquelin.stm32,
philippe.cornu, raphael.gallais-pou, yannick.fertre, linux-sunxi,
samuel, wens, jyri.sarha, tomi.valkeinen, hansg, dmitry.osipenko,
gurchetansingh, olvaffe, michal.simek, harry.wentland, siqueira,
sunpeng.li, linux, alexandre.belloni, claudiu.beznea, dharma.b,
manikandan.m, nicolas.ferre, alim.akhtar, inki.dae, krzk,
kyungmin.park, linux-samsung-soc, sw0312.kim, p.zabel,
angelogioacchino.delregno, chunkuang.hu, matthias.bgg, dakr,
lyude, nouveau, geert+renesas, kieran.bingham+renesas,
laurent.pinchart+renesas, linux-renesas-soc, magnus.damm,
tomi.valkeinen+renesas, biju.das.jz, dave.stevenson, kernel-list,
mcanal, bcm-kernel-feedback-list, zack.rusin, jonathanh,
linux-tegra, mperttunen, thierry.reding
This is a follow-up to the bridge reset removal series, and part of a
larger effort to remove the reset hook from all KMS objects.
The plane reset hook is overloaded: it is called both at probe time
to create the initial software state and during suspend/resume to
reset hardware and software state. These two roles have different
requirements, and the reset hook is not fallible, making error
handling difficult for the initial state allocation path.
While reset has the semantics to reset both the software and hardware
state, the vast majority of implementations and all the helpers only
reset the software state, making them equivalent to
atomic_create_state in practice. The atomic_create_state hook makes
this explicit: it only allocates and initializes a pristine state
without any side effect, and returns the state pointer or an ERR_PTR
on failure.
This series first adds the necessary infrastructure in the simple-kms
and GEM atomic helpers, then converts all 51 plane drivers tree-wide
from the reset hook to atomic_create_state. The conversions were done
using a combination of Coccinelle semantic patches and manual
adjustments. Once all drivers are converted, the old helpers and the
reset hook itself are removed from struct drm_plane_funcs.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Maxime Ripard (60):
drm/simple-kms: Add create_plane_state hook
drm/gem-atomic-helper: Create drm_gem_create_shadow_plane_state()
drm/gem-atomic-helper: Convert simple-kms shadow helpers to create_plane_state
drm/gem-atomic-helper: Switch DRM_GEM_SHADOW_PLANE_FUNCS to atomic_create_state
drm/gem-atomic-helper: Remove drm_gem_reset_shadow_plane()
drm/sysfb: Convert to atomic_create_state
drm/simple-kms: Switch to atomic_create_state
drm/ssd130x: Convert to atomic_create_state
drm/st7920: Convert to atomic_create_state
drm/appletbdrm: Convert to atomic_create_state
drm/vkms: Convert to atomic_create_state
drm/gem-atomic-helper: Remove __drm_gem_reset_shadow_plane()
drm/amdgpu: Convert to atomic_create_state
drm/hdlcd: Convert to atomic_create_state
drm/fsl-dcu: Convert to atomic_create_state
drm/hisilicon/kirin: Convert to atomic_create_state
drm/imx/dc: Convert to atomic_create_state
drm/imx/dcss: Convert to atomic_create_state
drm/ingenic: Convert to atomic_create_state
drm/kmb: Convert to atomic_create_state
drm/logicvc: Convert to atomic_create_state
drm/loongson: Convert to atomic_create_state
drm/meson: Convert to atomic_create_state
drm/msm/mdp4: Convert to atomic_create_state
drm/lcdif: Convert to atomic_create_state
drm/mxsfb: Convert to atomic_create_state
drm/qxl: Convert to atomic_create_state
drm/rockchip: Convert to atomic_create_state
drm/sprd: Convert to atomic_create_state
drm/sti: Convert to atomic_create_state
drm/stm: Convert to atomic_create_state
drm/sun4i: sun8i: Convert to atomic_create_state
drm/tests: kunit: Convert to atomic_create_state
drm/tilcdc: Convert to atomic_create_state
drm/vboxvideo: Convert to atomic_create_state
drm/virtio: Convert to atomic_create_state
drm/xlnx: Convert to atomic_create_state
drm/atomic-state-helper: Remove drm_atomic_helper_plane_reset()
drm/amdgpu_dm: Convert to atomic_create_state
drm/komeda: Convert to atomic_create_state
drm/malidp: Convert to atomic_create_state
drm/armada: Convert to atomic_create_state
drm/atmel-hlcdc: Convert to atomic_create_state
drm/exynos: Convert to atomic_create_state
drm/imx/ipuv3: Convert to atomic_create_state
drm/mediatek: Convert to atomic_create_state
drm/msm/dpu1: Convert to atomic_create_state
drm/msm/mdp5: Convert to atomic_create_state
drm/nouveau: Convert to atomic_create_state
drm/omap: Convert to atomic_create_state
drm/rcar-du: Convert to atomic_create_state
drm/rz-du: Convert to atomic_create_state
drm/shmobile: Convert to atomic_create_state
drm/sun4i: layer: Convert to atomic_create_state
drm/vc4: Convert to atomic_create_state
drm/verisilicon: Convert to atomic_create_state
drm/vmwgfx: Convert to atomic_create_state
drm/atomic-state-helper: Remove __drm_atomic_helper_plane_reset()
drm/tegra: Convert to atomic_create_state
drm/plane: Remove reset
drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 +-
.../drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 17 ++--
drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 18 ++--
drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +-
drivers/gpu/drm/arm/malidp_planes.c | 12 ++-
drivers/gpu/drm/armada/armada_overlay.c | 39 ++++----
drivers/gpu/drm/armada/armada_plane.c | 15 ++--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 27 +++---
drivers/gpu/drm/drm_atomic_state_helper.c | 41 ---------
drivers/gpu/drm/drm_gem_atomic_helper.c | 100 +++++++++++----------
drivers/gpu/drm/drm_mode_config.c | 4 +-
drivers/gpu/drm/drm_simple_kms_helper.c | 11 +--
drivers/gpu/drm/exynos/exynos_drm_plane.c | 22 ++---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 2 +-
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +-
drivers/gpu/drm/imx/dc/dc-plane.c | 2 +-
drivers/gpu/drm/imx/dcss/dcss-plane.c | 2 +-
drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 19 ++--
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +-
drivers/gpu/drm/ingenic/ingenic-ipu.c | 2 +-
drivers/gpu/drm/kmb/kmb_plane.c | 2 +-
drivers/gpu/drm/logicvc/logicvc_layer.c | 2 +-
drivers/gpu/drm/loongson/lsdc_plane.c | 2 +-
drivers/gpu/drm/mediatek/mtk_plane.c | 22 ++---
drivers/gpu/drm/meson/meson_overlay.c | 2 +-
drivers/gpu/drm/meson/meson_plane.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 ++--
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 16 ++--
drivers/gpu/drm/mxsfb/lcdif_kms.c | 2 +-
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 15 ++--
drivers/gpu/drm/omapdrm/omap_plane.c | 13 ++-
drivers/gpu/drm/qxl/qxl_display.c | 4 +-
drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c | 15 ++--
drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 15 ++--
drivers/gpu/drm/renesas/rz-du/rzg2l_du_vsp.c | 15 ++--
drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c | 15 ++--
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2 +-
drivers/gpu/drm/sitronix/st7920.c | 12 +--
drivers/gpu/drm/solomon/ssd130x.c | 12 +--
drivers/gpu/drm/sprd/sprd_dpu.c | 2 +-
drivers/gpu/drm/sti/sti_cursor.c | 2 +-
drivers/gpu/drm/sti/sti_gdp.c | 2 +-
drivers/gpu/drm/sti/sti_hqvdp.c | 2 +-
drivers/gpu/drm/stm/ltdc.c | 2 +-
drivers/gpu/drm/sun4i/sun4i_layer.c | 12 ++-
drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 2 +-
drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 +-
drivers/gpu/drm/sysfb/drm_sysfb_helper.h | 4 +-
drivers/gpu/drm/sysfb/drm_sysfb_modeset.c | 30 +++++--
drivers/gpu/drm/tegra/plane.c | 28 +++---
drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 +-
drivers/gpu/drm/tilcdc/tilcdc_plane.c | 2 +-
drivers/gpu/drm/tiny/appletbdrm.c | 12 +--
drivers/gpu/drm/vboxvideo/vbox_mode.c | 2 +-
drivers/gpu/drm/vc4/vc4_plane.c | 15 ++--
drivers/gpu/drm/verisilicon/vs_cursor_plane.c | 2 +-
drivers/gpu/drm/verisilicon/vs_plane.c | 14 ++-
drivers/gpu/drm/verisilicon/vs_plane.h | 2 +-
drivers/gpu/drm/verisilicon/vs_primary_plane.c | 2 +-
drivers/gpu/drm/virtio/virtgpu_plane.c | 2 +-
drivers/gpu/drm/vkms/vkms_plane.c | 15 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 17 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 +-
drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
include/drm/drm_atomic_state_helper.h | 3 -
include/drm/drm_gem_atomic_helper.h | 13 +--
include/drm/drm_plane.h | 12 ---
include/drm/drm_simple_kms_helper.h | 1 +
74 files changed, 331 insertions(+), 422 deletions(-)
---
base-commit: 671b7825dbfe9ea6e3ad3001003aeee0df48d1b5
change-id: 20260629-drm-no-more-plane-reset-04950f42e07f
Best regards,
--
Maxime Ripard <mripard@kernel.org>
^ permalink raw reply
* Re: [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation
From: Joseph Qi @ 2026-07-09 11:01 UTC (permalink / raw)
To: Pankaj Gupta; +Cc: Christoph Hellwig, virtualization, linux-kernel, Baokun Li
In-Reply-To: <CAM9Jb+hPmV4KF5xAy0=CPy6yh4=9=aACmLbZ3G5daz7QuAH3hQ@mail.gmail.com>
On 7/9/26 6:58 PM, Pankaj Gupta wrote:
>> On 7/9/26 12:24 AM, Christoph Hellwig wrote:
>>> On Wed, Jul 08, 2026 at 08:42:38PM +0800, Joseph Qi wrote:
>>>> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
>>>> index 4176046627beb..081370aac6317 100644
>>>> --- a/drivers/nvdimm/nd_virtio.c
>>>> +++ b/drivers/nvdimm/nd_virtio.c
>>>> @@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
>>>> if (bio && bio->bi_iter.bi_sector != -1) {
>>>> struct bio *child = bio_alloc(bio->bi_bdev, 0,
>>>> REQ_OP_WRITE | REQ_PREFLUSH,
>>>> - GFP_ATOMIC);
>>>> + GFP_NOIO);
>>>>
>>>> if (!child)
>>>> return -ENOMEM;
>>>
>>> This NULL check can go away now, and probaby should to avoid confusion.
>>>
>>> Also bio_alloc allocates from fs_bio_set, so if the incoming bio
>>> is from that, we can still deadlock. We'll need a separate bio_set
>>> for this to be deadlock free. disk->bio_split isn't otherwise
>>> used for the drivers/nvdimm/ driverss, so you might be able to
>>> repurpose that if you want to be creative.
>>
>> Thanks for pointing this out.
>>
>> Seems it is more proper to use a driver-private bio_set instead of
>> repurposing bd_disk->bio_split, like raid5-ppl flush_bs.
>>
>> Pankaj, what's your opinion?
>
> Hi Joseph,
>
> Thank you for the patch!
>
> I am not very familiar with the internals of fs_bio_set, so I do not
> have a strong opinion on the implementation details. However, a
> driver-private bio_set sounds reasonable to me, and I would go with
> Christoph's suggestion.
>
Fine, I'll send out v2 later to address this.
Thanks,
Joseph
^ permalink raw reply
* Re: [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation
From: Pankaj Gupta @ 2026-07-09 10:58 UTC (permalink / raw)
To: Joseph Qi; +Cc: Christoph Hellwig, virtualization, linux-kernel, Baokun Li
In-Reply-To: <55f7befb-5ef3-4ede-a3ed-4968864c45b7@linux.alibaba.com>
> On 7/9/26 12:24 AM, Christoph Hellwig wrote:
> > On Wed, Jul 08, 2026 at 08:42:38PM +0800, Joseph Qi wrote:
> >> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> >> index 4176046627beb..081370aac6317 100644
> >> --- a/drivers/nvdimm/nd_virtio.c
> >> +++ b/drivers/nvdimm/nd_virtio.c
> >> @@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
> >> if (bio && bio->bi_iter.bi_sector != -1) {
> >> struct bio *child = bio_alloc(bio->bi_bdev, 0,
> >> REQ_OP_WRITE | REQ_PREFLUSH,
> >> - GFP_ATOMIC);
> >> + GFP_NOIO);
> >>
> >> if (!child)
> >> return -ENOMEM;
> >
> > This NULL check can go away now, and probaby should to avoid confusion.
> >
> > Also bio_alloc allocates from fs_bio_set, so if the incoming bio
> > is from that, we can still deadlock. We'll need a separate bio_set
> > for this to be deadlock free. disk->bio_split isn't otherwise
> > used for the drivers/nvdimm/ driverss, so you might be able to
> > repurpose that if you want to be creative.
>
> Thanks for pointing this out.
>
> Seems it is more proper to use a driver-private bio_set instead of
> repurposing bd_disk->bio_split, like raid5-ppl flush_bs.
>
> Pankaj, what's your opinion?
Hi Joseph,
Thank you for the patch!
I am not very familiar with the internals of fs_bio_set, so I do not
have a strong opinion on the implementation details. However, a
driver-private bio_set sounds reasonable to me, and I would go with
Christoph's suggestion.
Best regards,
Pankaj
>
> Thanks,
> Joseph
^ permalink raw reply
* Re: [PATCH] iommu/virtio: reject short event buffers
From: Jean-Philippe Brucker @ 2026-07-09 9:55 UTC (permalink / raw)
To: raoxu; +Cc: joro, will, robin.murphy, virtualization, iommu, linux-kernel
In-Reply-To: <6800BE0812AF0897+20260706083148.788991-1-raoxu@uniontech.com>
On Mon, Jul 06, 2026 at 04:31:48PM +0800, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
>
> The event queue uses fixed-size buffers for struct viommu_event. The
> device-reported used length is currently only checked for oversized
> buffers, so a short used buffer can still be passed to the fault
> handler.
Thank you for the patch. The buffer is allowed to be smaller than struct
virtio_iommu_fault, because valid fields depend on flags:
struct virtio_iommu_fault {
u8 reason;
u8 reserved[3];
le32 flags;
le32 endpoint;
le32 reserved1;
le64 address;
};
#define VIRTIO_IOMMU_FAULT_F_READ (1 << 0)
#define VIRTIO_IOMMU_FAULT_F_WRITE (1 << 1)
#define VIRTIO_IOMMU_FAULT_F_ADDRESS (1 << 8)
If F_ADDRESS isn't set, the address field is invalid. The spec (Virtio v1.3)
says in 5.13.6.9.2 "Device Requirements: Fault reporting"
"The device MAY omit setting VIRTIO_IOMMU_FAULT_F_ADDRESS and writing
address in any fault report, regardless of the reason."
I think the current check aims to catch newer implementations that would
use an extended virtio_iommu_fault to report recoverable faults, but I'm
not sure anymore.
Overall the driver assumes that the IOMMU device is well behaved. If we
want to add sanity checks, maybe viommu_fault_handler() should check len
depending on flags. But I suspect more work is needed if we change the
security model to not trust the device.
Thanks,
Jean
>
> In that case the handler parses fields that were not written by the
> device for this event, potentially using stale data from a previous
> event and reporting a bogus fault.
>
> Reject any event buffer whose used length is not exactly the expected
> event size. Still recycle the buffer back to the event queue so a bad
> event does not permanently shrink the queue.
>
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> ---
> drivers/iommu/virtio-iommu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 587fc13197f1..3ace4a6dd02a 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -635,7 +635,7 @@ static void viommu_event_handler(struct virtqueue *vq)
> struct viommu_dev *viommu = vq->vdev->priv;
>
> while ((evt = virtqueue_get_buf(vq, &len)) != NULL) {
> - if (len > sizeof(*evt)) {
> + if (len != sizeof(*evt)) {
> dev_err(viommu->dev,
> "invalid event buffer (len %u != %zu)\n",
> len, sizeof(*evt));
> --
> 2.50.1
>
^ permalink raw reply
* Re: [PATCH net v2 2/2] vsock/test: add test for small packets under pressure
From: Stefano Garzarella @ 2026-07-09 9:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez,
Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, kvm,
Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang
In-Reply-To: <20260708065853-mutt-send-email-mst@kernel.org>
On Wed, Jul 08, 2026 at 06:59:41AM -0400, Michael S. Tsirkin wrote:
>On Wed, Jul 08, 2026 at 12:29:04PM +0200, Stefano Garzarella wrote:
>> From: Stefano Garzarella <sgarzare@redhat.com>
>>
>> Add a test that sends 2 MB of data using randomly sized small packets
>> (129-512 bytes) over a SOCK_STREAM connection. Packets above
>> GOOD_COPY_LEN (128) bypass the in-place coalescing in recv_enqueue(),
>> forcing each one into its own skb.
>>
>> Without receive queue collapsing, the per-skb overhead eventually
>> exceeds buf_alloc and the connection is reset. The test verifies
>> that all data arrives and that content integrity is preserved.
>>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>
>maybe cut down SO_VM_SOCKETS_BUFFER_SIZE? will make it easier to
>trigger?
Currently, with the default value, the trigger is practically immediate
for packets between 129 and 512 bytes, but yes, a smaller buffer size
certainly makes this effect even more pronounced.
>
>anyway
>
>Acked-by: Michael S. Tsirkin <mst@redhat.com>
Thanks!
If I need to post a v3, I'll add it; otherwise, I guess we can leave it
as is or send a follow-up for net-next.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH net v2 1/2] vsock/virtio: collapse receive queue under memory pressure
From: Stefano Garzarella @ 2026-07-09 8:54 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, Jason Wang, Xuan Zhuo, Eric Dumazet, Eugenio Pérez,
Simon Horman, Stefan Hajnoczi, David S. Miller, linux-kernel, kvm,
Paolo Abeni, virtualization, Jakub Kicinski, Jason Wang, stable,
Brien Oberstein
In-Reply-To: <20260708065947-mutt-send-email-mst@kernel.org>
On Wed, Jul 08, 2026 at 07:00:00AM -0400, Michael S. Tsirkin wrote:
>On Wed, Jul 08, 2026 at 12:29:03PM +0200, 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 proactively from when the number of skb queued
>> is close to exceeding the overhead budget.
>>
>> 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.
>>
>> Walking a large queue may take a significant amount of time and cache
>> misses, causing traffic burstiness. To limit this, the collapse stops
>> once enough room is freed for this packet and the next one, but may
>> opportunistically free more to fill each collapsed skb to capacity.
>>
>> [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>
>
>
>this is the right approach
Yeah, I have a follow up to start to use skb->truesize, etc. but I guess
more net-next material.
>
>Acked-by: Michael S. Tsirkin <mst@redhat.com>
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH] virtio-pmem: use GFP_NOIO for flush bio allocation
From: Joseph Qi @ 2026-07-09 3:13 UTC (permalink / raw)
To: Christoph Hellwig, Pankaj Gupta; +Cc: virtualization, linux-kernel, Baokun Li
In-Reply-To: <20260708162435.GB2502@lst.de>
On 7/9/26 12:24 AM, Christoph Hellwig wrote:
> On Wed, Jul 08, 2026 at 08:42:38PM +0800, Joseph Qi wrote:
>> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
>> index 4176046627beb..081370aac6317 100644
>> --- a/drivers/nvdimm/nd_virtio.c
>> +++ b/drivers/nvdimm/nd_virtio.c
>> @@ -117,7 +117,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
>> if (bio && bio->bi_iter.bi_sector != -1) {
>> struct bio *child = bio_alloc(bio->bi_bdev, 0,
>> REQ_OP_WRITE | REQ_PREFLUSH,
>> - GFP_ATOMIC);
>> + GFP_NOIO);
>>
>> if (!child)
>> return -ENOMEM;
>
> This NULL check can go away now, and probaby should to avoid confusion.
>
> Also bio_alloc allocates from fs_bio_set, so if the incoming bio
> is from that, we can still deadlock. We'll need a separate bio_set
> for this to be deadlock free. disk->bio_split isn't otherwise
> used for the drivers/nvdimm/ driverss, so you might be able to
> repurpose that if you want to be creative.
Thanks for pointing this out.
Seems it is more proper to use a driver-private bio_set instead of
repurposing bd_disk->bio_split, like raid5-ppl flush_bs.
Pankaj, what's your opinion?
Thanks,
Joseph
^ permalink raw reply
* Re: [PATCH 13/13] mm/mremap: convert mremap code to use vma_flags_t
From: Zi Yan @ 2026-07-09 2:28 UTC (permalink / raw)
To: Lorenzo Stoakes, Lance Yang
Cc: akpm, tsbogend, maddy, mpe, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, l.stach, inki.dae, sw0312.kim,
kyungmin.park, krzk, peter.griffin, jani.nikula, joonas.lahtinen,
rodrigo.vivi, tursulin, robin.clark, lumag, lyude, dakr,
tomi.valkeinen, hjc, heiko, andy.yan, thierry.reding, mperttunen,
jonathanh, kraxel, dmitry.osipenko, zack.rusin, matthew.brost,
thomas.hellstrom, oleksandr_andrushchenko, deller, bcrl, viro,
brauner, muchun.song, osalvador, david, baolin.wang, liam, npache,
ryan.roberts, dev.jain, baohua, hughd, vbabka, rppt, surenb,
mhocko, jannh, pfalcato, kees, perex, tiwai, 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: <akaJx8Zt8kazlrjq@lucifer>
On Thu Jul 2, 2026 at 12:07 PM EDT, Lorenzo Stoakes wrote:
> On Thu, Jul 02, 2026 at 09:49:47PM +0800, Lance Yang wrote:
>>
>> On Mon, Jun 29, 2026 at 08:25:36PM +0100, Lorenzo Stoakes wrote:
>> >Replace use of the legacy vm_flags_t flags with vma_flags_t values
>> >throughout the mremap logic.
>> >
>> >Additionally update comments to reflect the changes to be consistent.
>> >
>> >No functional change intended.
>> >
>> >Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>> >---
>>
>> The vm_flags_set() cases below spell out vma_start_write(), but the
>> vm_flags_clear() cases don't?
>
> Yep as I said elsewhere, implicitly taking the lock is terrible and me doing
> this is completely on purpose to get rid of that :)
>
> But I haven't been clear enough clearly, so I should put the argument as to why
> that's ok in the commit message.
>
> Will do so on respin.
How about also add a comment to vma_clear*() telling us a lock is not
needed and why like you explained a lock is needed for vma_set*()?
This asymmetry could confuse people.
This patch looks good to me.
Reviewed-by: Zi Yan <ziy@nvidia.com>
--
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH 12/13] mm/mprotect: convert mprotect code to use vma_flags_t
From: Zi Yan @ 2026-07-09 2:16 UTC (permalink / raw)
To: Lorenzo Stoakes, Lance Yang
Cc: akpm, tsbogend, maddy, mpe, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, l.stach, inki.dae, sw0312.kim,
kyungmin.park, krzk, peter.griffin, jani.nikula, joonas.lahtinen,
rodrigo.vivi, tursulin, robin.clark, lumag, lyude, dakr,
tomi.valkeinen, hjc, heiko, andy.yan, thierry.reding, mperttunen,
jonathanh, kraxel, dmitry.osipenko, zack.rusin, matthew.brost,
thomas.hellstrom, oleksandr_andrushchenko, deller, bcrl, viro,
brauner, muchun.song, osalvador, david, baolin.wang, liam, npache,
ryan.roberts, dev.jain, baohua, hughd, vbabka, rppt, surenb,
mhocko, jannh, pfalcato, kees, perex, tiwai, 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: <akaIfhfbTCAdJm3H@lucifer>
On Thu Jul 2, 2026 at 11:53 AM EDT, Lorenzo Stoakes wrote:
> On Thu, Jul 02, 2026 at 12:09:17AM +0800, Lance Yang wrote:
>>
>> On Mon, Jun 29, 2026 at 08:25:35PM +0100, Lorenzo Stoakes wrote:
>> >Replace use of the legacy vm_flags_t flags with vma_flags_t values
>> >throughout the mprotect logic.
>> >
>> >Note that we retain the legacy vm_flags_t bit shifting code in
>> >do_mprotect_key(), deferring a vma_flags_t approach to this for the time
>> >being.
>> >
>> >Additionally update comments to reflect the changes to be consistent.
>> >
>> >No functional change intended.
>> >
>> >Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>> >---
>> > mm/mprotect.c | 16 ++++++++--------
>> > 1 file changed, 8 insertions(+), 8 deletions(-)
>> >
>> >diff --git a/mm/mprotect.c b/mm/mprotect.c
>> >index 9cbf932b028c..c9504b2a2525 100644
>> >--- a/mm/mprotect.c
>> >+++ b/mm/mprotect.c
>> >@@ -40,7 +40,7 @@
>> >
>> > static bool maybe_change_pte_writable(struct vm_area_struct *vma, pte_t pte)
>> > {
>> >- if (WARN_ON_ONCE(!(vma->vm_flags & VM_WRITE)))
>> >+ if (WARN_ON_ONCE(!vma_test(vma, VMA_WRITE_BIT)))
>> > return false;
>> >
>> > /* Don't touch entries that are not even readable. */
>> >@@ -97,7 +97,7 @@ static bool can_change_shared_pte_writable(struct vm_area_struct *vma,
>> > bool can_change_pte_writable(struct vm_area_struct *vma, unsigned long addr,
>> > pte_t pte)
>> > {
>> >- if (!(vma->vm_flags & VM_SHARED))
>> >+ if (!vma_test(vma, VMA_SHARED_BIT))
>> > return can_change_private_pte_writable(vma, addr, pte);
>> >
>> > return can_change_shared_pte_writable(vma, pte);
>> >@@ -194,7 +194,7 @@ static __always_inline void set_write_prot_commit_flush_ptes(struct vm_area_stru
>> > {
>> > bool set_write;
>> >
>> >- if (vma->vm_flags & VM_SHARED) {
>> >+ if (vma_test(vma, VMA_SHARED_BIT)) {
>> > set_write = can_change_shared_pte_writable(vma, ptent);
>> > prot_commit_flush_ptes(vma, addr, ptep, oldpte, ptent, nr_ptes,
>> > /* idx = */ 0, set_write, tlb);
>> >@@ -811,8 +811,8 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
>> > vm_unacct_memory(nrpages);
>> >
>> > /*
>> >- * Private VM_LOCKED VMA becoming writable: trigger COW to avoid major
>> >- * fault on access.
>> >+ * Private VMA_LOCKED_BIT VMA becoming writable: trigger COW to avoid
>> >+ * major fault on access.
>> > */
>> > if (vma_flags_test(&new_vma_flags, VMA_WRITE_BIT) &&
>> > vma_flags_test(&old_vma_flags, VMA_LOCKED_BIT) &&
>> >@@ -886,7 +886,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
>> > goto out;
>> > start = vma->vm_start;
>> > error = -EINVAL;
>> >- if (!(vma->vm_flags & VM_GROWSDOWN))
>> >+ if (!vma_test(vma, VMA_GROWSDOWN_BIT))
>> > goto out;
>> > } else {
>> > if (vma->vm_start > start)
>> >@@ -894,7 +894,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
>> > if (unlikely(grows & PROT_GROWSUP)) {
>> > end = vma->vm_end;
>> > error = -EINVAL;
>> >- if (!(vma->vm_flags & VM_GROWSUP))
>> >+ if (!vma_test(vma, VMA_GROWSUP_BIT))
>>
>> IIUC, should this be
>>
>> if (!vma_test_single_mask(vma, VMA_GROWSUP))
>>
>> instead?
>>
>> #elif defined(CONFIG_PARISC)
>> #define VM_GROWSUP INIT_VM_FLAG(GROWSUP)
>> ...
>> #ifndef VM_GROWSUP
>> #define VM_GROWSUP VM_NONE
>> ...
>>
>> VM_GROWSUP is only defined as GROWSUP on parisc and becomes VM_NONE
>> elsewhere. But VMA_GROWSUP_BIT is the raw ARCH_1 bit, which is also used
>> for other arch-specific VMA flags:
>>
>> DECLARE_VMA_BIT_ALIAS(SAO, ARCH_1), /* Strong Access Ordering (powerpc) */
>> DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1), /* parisc */
>> DECLARE_VMA_BIT_ALIAS(SPARC_ADI, ARCH_1), /* sparc64 */
>> DECLARE_VMA_BIT_ALIAS(ARM64_BTI, ARCH_1), /* arm64 */
>> DECLARE_VMA_BIT_ALIAS(ARCH_CLEAR, ARCH_1), /* sparc64, arm64 */
>> DECLARE_VMA_BIT_ALIAS(MAPPED_COPY, ARCH_1), /* !CONFIG_MMU */
>>
>> Other vma_test() changes look fine to me: just fixed INIT_VM_FLAG()
>> masks matching their VMA_*_BIT :)
>
> Thanks you're right, will fix!
>
> Again I swear I ran claude on all of this so it's failing me here :)
>
Is it better to add something like below to avoid misuse these mutually
exclusive bit aliases?
An example for VMA_GROWSUP_BIT:
#if defined(CONFIG_PARISC)
DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1), /* parisc */
#else
/* make VMA_GROWSUP_BIT a build bug on */
#endif
Hmm, these VMA_*_BIT are enum items, so the above might not be possible.
An alternative is to only define them for the corresponding config and
you will get build errors when trying to use them directly and the
config is not enabled. Otherwise, misuses like "vma_test(vma,
VMA_GROWSUP_BIT)" is harder to uncover.
--
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH 11/13] mm/mlock: convert mlock code to use vma_flags_t
From: Zi Yan @ 2026-07-09 2:01 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, 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: <2db16db81538355ca65f778c246d2381c673cad4.1782760670.git.ljs@kernel.org>
On Mon Jun 29, 2026 at 3:25 PM EDT, Lorenzo Stoakes wrote:
> Replace use of the legacy vm_flags_t flags with vma_flags_t values
> throughout the mlock logic.
>
> Additionally update comments to reflect the changes to be consistent.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
> mm/mlock.c | 86 +++++++++++++++++++++++++++++-------------------------
> 1 file changed, 46 insertions(+), 40 deletions(-)
>
LGTM. What confused me when I was reading the code is VMA_LOCKED_MASK
and VMA_LOCKED_BIT, since both share the same prefix VMA_LOCKED. Before
looking at the definition of VMA_LOCKED_MASK, I was wondering when to
use _BIT or _MASK. But it is just a naming thing.
Anyway,
Reviewed-by: Zi Yan <ziy@nvidia.com>
--
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