Linux virtualization list
 help / color / mirror / Atom feed
* Re: [PATCH net v3 1/2] vsock/virtio: reset connection on receiving queue overflow
From: Stefano Garzarella @ 2026-05-14 14:57 UTC (permalink / raw)
  To: netdev
  Cc: Xuan Zhuo, Michael S. Tsirkin, Eugenio Pérez, linux-kernel,
	Simon Horman, Paolo Abeni, Jakub Kicinski, Jason Wang, kvm,
	Stefan Hajnoczi, virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <20260513105417.56761-2-sgarzare@redhat.com>

On Wed, May 13, 2026 at 12:54:16PM +0200, Stefano Garzarella wrote:
>From: Stefano Garzarella <sgarzare@redhat.com>
>
>When there is no more space to queue an incoming packet, the packet is
>silently dropped. This causes data loss without any notification to
>either peer, since there is no retransmission.
>
>Under normal circumstances, this should never happen. However, it could
>happen if the other peer doesn't respect the credit, or if the skb
>overhead, which we recently began to take into account with commit
>059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue"),
>is too high.
>
>Fix this by resetting the connection and setting the local socket error
>to ENOBUFS when virtio_transport_recv_enqueue() can no longer queue a
>packet, so both peers are explicitly notified of the failure rather than
>silently losing data.
>
>Fixes: ae6fcfbf5f03 ("vsock/virtio: discard packets if credit is not respected")
>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 989cc252d3d3..4a4ac69d1ad1 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1350,7 +1350,7 @@ virtio_transport_recv_connecting(struct sock *sk,
> 	return err;
> }
>
>-static void
>+static bool
> virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> 			      struct sk_buff *skb)
> {
>@@ -1365,10 +1365,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> 	spin_lock_bh(&vvs->rx_lock);
>
> 	can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
>-	if (!can_enqueue) {
>-		free_pkt = true;
>+	if (!can_enqueue)
> 		goto out;
>-	}
>
> 	if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
> 		vvs->msg_count++;
>@@ -1408,6 +1406,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> 	spin_unlock_bh(&vvs->rx_lock);
> 	if (free_pkt)
> 		kfree_skb(skb);
>+
>+	return can_enqueue;
> }
>
> static int
>@@ -1420,7 +1420,16 @@ virtio_transport_recv_connected(struct sock *sk,
>
> 	switch (le16_to_cpu(hdr->op)) {
> 	case VIRTIO_VSOCK_OP_RW:
>-		virtio_transport_recv_enqueue(vsk, skb);
>+		if (!virtio_transport_recv_enqueue(vsk, skb)) {
>+			/* There is no more space to queue the packet, so let's
>+			 * close the connection; otherwise, we'll lose data.
>+			 */
>+			(void)virtio_transport_reset(vsk, skb);
>+			sk->sk_state = TCP_CLOSE;
>+			sk->sk_err = ENOBUFS;
>+			sk_error_report(sk);

sashiko reported some issues related to setting TCP_CLOSE state and not 
removing the socket from the connect table:
https://sashiko.dev/#/patchset/20260513105417.56761-1-sgarzare%40redhat.com

I'll change this by calling virtio_transport_do_close() and 
vsock_remove_sock() in the next version.

Stefano

>+			break;
>+		}
> 		vsock_data_ready(sk);
> 		return err;
> 	case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
>-- 
>2.54.0
>


^ permalink raw reply

* Re: [PATCH net v3 1/2] vsock/virtio: reset connection on receiving queue overflow
From: Michael S. Tsirkin @ 2026-05-14 15:16 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: netdev, Xuan Zhuo, Eugenio Pérez, linux-kernel, Simon Horman,
	Paolo Abeni, Jakub Kicinski, Jason Wang, kvm, Stefan Hajnoczi,
	virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <agXh36gUe1K6__Bk@sgarzare-redhat>

On Thu, May 14, 2026 at 04:57:16PM +0200, Stefano Garzarella wrote:
> On Wed, May 13, 2026 at 12:54:16PM +0200, Stefano Garzarella wrote:
> > From: Stefano Garzarella <sgarzare@redhat.com>
> > 
> > When there is no more space to queue an incoming packet, the packet is
> > silently dropped. This causes data loss without any notification to
> > either peer, since there is no retransmission.
> > 
> > Under normal circumstances, this should never happen. However, it could
> > happen if the other peer doesn't respect the credit, or if the skb
> > overhead, which we recently began to take into account with commit
> > 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue"),
> > is too high.
> > 
> > Fix this by resetting the connection and setting the local socket error
> > to ENOBUFS when virtio_transport_recv_enqueue() can no longer queue a
> > packet, so both peers are explicitly notified of the failure rather than
> > silently losing data.
> > 
> > Fixes: ae6fcfbf5f03 ("vsock/virtio: discard packets if credit is not respected")
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> > net/vmw_vsock/virtio_transport_common.c | 19 ++++++++++++++-----
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index 989cc252d3d3..4a4ac69d1ad1 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -1350,7 +1350,7 @@ virtio_transport_recv_connecting(struct sock *sk,
> > 	return err;
> > }
> > 
> > -static void
> > +static bool
> > virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > 			      struct sk_buff *skb)
> > {
> > @@ -1365,10 +1365,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > 	spin_lock_bh(&vvs->rx_lock);
> > 
> > 	can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
> > -	if (!can_enqueue) {
> > -		free_pkt = true;
> > +	if (!can_enqueue)
> > 		goto out;
> > -	}
> > 
> > 	if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
> > 		vvs->msg_count++;
> > @@ -1408,6 +1406,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > 	spin_unlock_bh(&vvs->rx_lock);
> > 	if (free_pkt)
> > 		kfree_skb(skb);
> > +
> > +	return can_enqueue;
> > }
> > 
> > static int
> > @@ -1420,7 +1420,16 @@ virtio_transport_recv_connected(struct sock *sk,
> > 
> > 	switch (le16_to_cpu(hdr->op)) {
> > 	case VIRTIO_VSOCK_OP_RW:
> > -		virtio_transport_recv_enqueue(vsk, skb);
> > +		if (!virtio_transport_recv_enqueue(vsk, skb)) {
> > +			/* There is no more space to queue the packet, so let's
> > +			 * close the connection; otherwise, we'll lose data.
> > +			 */
> > +			(void)virtio_transport_reset(vsk, skb);
> > +			sk->sk_state = TCP_CLOSE;
> > +			sk->sk_err = ENOBUFS;
> > +			sk_error_report(sk);
> 
> sashiko reported some issues related to setting TCP_CLOSE state and not
> removing the socket from the connect table:
> https://sashiko.dev/#/patchset/20260513105417.56761-1-sgarzare%40redhat.com
> 
> I'll change this by calling virtio_transport_do_close() and
> vsock_remove_sock() in the next version.
> 
> Stefano
> 
> > +			break;
> > +		}
> > 		vsock_data_ready(sk);
> > 		return err;
> > 	case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
> > -- 
> > 2.54.0
> > 


And so the bag of hacks grows. I feel this is energy not well spent.
Please, let us fix this properly *first*. And then worry about how to
backport.  Maybe it will not be so terrible to backport after all.


^ permalink raw reply

* Re: [PATCH v7 02/31] mm: page_alloc: propagate PageReported flag across buddy splits
From: Gregory Price @ 2026-05-14 15:32 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <20260514104735-mutt-send-email-mst@kernel.org>

On Thu, May 14, 2026 at 10:48:28AM -0400, Michael S. Tsirkin wrote:
> On Thu, May 14, 2026 at 07:51:03AM -0400, Gregory Price wrote:
> > On Tue, May 12, 2026 at 05:05:16PM -0400, Michael S. Tsirkin wrote:
> > > When a reported free page is split via expand() to satisfy a
> > > smaller allocation, the sub-pages placed back on the free lists
> > > lose the PageReported flag.  This means they will be unnecessarily
> > > re-reported to the hypervisor in the next reporting cycle, wasting
> > > work.
> > > 
> > > While I was unable to quantify the performance difference, it is
> > > an obvious waste, even if small.
> > > 
> > > Propagate the PageReported flag to sub-pages during expand(),
> > > both in page_del_and_expand() and try_to_claim_block(), so
> > > that they are recognized as already-reported.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > Assisted-by: Claude:claude-opus-4-6
> > ... snip ...
> > > @@ -1731,9 +1740,10 @@ static __always_inline void page_del_and_expand(struct zone *zone,
> > >  						int high, int migratetype)
> > >  {
> > >  	int nr_pages = 1 << high;
> > > +	bool was_reported = page_reported(page);
> > >  
> > >  	__del_page_from_free_list(page, zone, high, migratetype);
> > > -	nr_pages -= expand(zone, page, low, high, migratetype);
> > > +	nr_pages -= expand(zone, page, low, high, migratetype, was_reported);
> > >  	account_freepages(zone, -nr_pages, migratetype);
> > >  }
> > >  
> > 
> > Maybe mildly out of scope but worth asking:  Are there other flags that
> > should be retained/propogated on a split?  If so, rather than pass
> > was_reported, should we just take a temporary copy of the page flags and
> > pass them all in?
> > 
> > ~Gregory
> 
> 
> Not that I can see, no.
> 

You do this exact thing again later :]

https://lore.kernel.org/linux-mm/9a22e0f9bbe1278913754db6df76e291a006181a.1778616612.git.mst@redhat.com/

~Gregory

^ permalink raw reply

* Re: [PATCH v7 02/31] mm: page_alloc: propagate PageReported flag across buddy splits
From: Michael S. Tsirkin @ 2026-05-14 15:34 UTC (permalink / raw)
  To: Gregory Price
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <agXrCinY2HmJmhT6@gourry-fedora-PF4VCD3F>

On Thu, May 14, 2026 at 11:32:26AM -0400, Gregory Price wrote:
> On Thu, May 14, 2026 at 10:48:28AM -0400, Michael S. Tsirkin wrote:
> > On Thu, May 14, 2026 at 07:51:03AM -0400, Gregory Price wrote:
> > > On Tue, May 12, 2026 at 05:05:16PM -0400, Michael S. Tsirkin wrote:
> > > > When a reported free page is split via expand() to satisfy a
> > > > smaller allocation, the sub-pages placed back on the free lists
> > > > lose the PageReported flag.  This means they will be unnecessarily
> > > > re-reported to the hypervisor in the next reporting cycle, wasting
> > > > work.
> > > > 
> > > > While I was unable to quantify the performance difference, it is
> > > > an obvious waste, even if small.
> > > > 
> > > > Propagate the PageReported flag to sub-pages during expand(),
> > > > both in page_del_and_expand() and try_to_claim_block(), so
> > > > that they are recognized as already-reported.
> > > > 
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > Assisted-by: Claude:claude-opus-4-6
> > > ... snip ...
> > > > @@ -1731,9 +1740,10 @@ static __always_inline void page_del_and_expand(struct zone *zone,
> > > >  						int high, int migratetype)
> > > >  {
> > > >  	int nr_pages = 1 << high;
> > > > +	bool was_reported = page_reported(page);
> > > >  
> > > >  	__del_page_from_free_list(page, zone, high, migratetype);
> > > > -	nr_pages -= expand(zone, page, low, high, migratetype);
> > > > +	nr_pages -= expand(zone, page, low, high, migratetype, was_reported);
> > > >  	account_freepages(zone, -nr_pages, migratetype);
> > > >  }
> > > >  
> > > 
> > > Maybe mildly out of scope but worth asking:  Are there other flags that
> > > should be retained/propogated on a split?  If so, rather than pass
> > > was_reported, should we just take a temporary copy of the page flags and
> > > pass them all in?
> > > 
> > > ~Gregory
> > 
> > 
> > Not that I can see, no.
> > 
> 
> You do this exact thing again later :]
> 
> https://lore.kernel.org/linux-mm/9a22e0f9bbe1278913754db6df76e291a006181a.1778616612.git.mst@redhat.com/
> 
> ~Gregory

I mean yes, reported and zeroed) Just zeroed does not exist at this
stage in the series.


^ permalink raw reply

* Re: [PATCH v7 02/31] mm: page_alloc: propagate PageReported flag across buddy splits
From: Gregory Price @ 2026-05-14 15:41 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <20260514113337-mutt-send-email-mst@kernel.org>

On Thu, May 14, 2026 at 11:34:14AM -0400, Michael S. Tsirkin wrote:
> On Thu, May 14, 2026 at 11:32:26AM -0400, Gregory Price wrote:
> > > > 
> > > > Maybe mildly out of scope but worth asking:  Are there other flags that
> > > > should be retained/propogated on a split?  If so, rather than pass
> > > > was_reported, should we just take a temporary copy of the page flags and
> > > > pass them all in?
> > > > 
> > > > ~Gregory
> > > 
> > > 
> > > Not that I can see, no.
> > > 
> > 
> > You do this exact thing again later :]
> > 
> > https://lore.kernel.org/linux-mm/9a22e0f9bbe1278913754db6df76e291a006181a.1778616612.git.mst@redhat.com/
> > 
> > ~Gregory
> 
> I mean yes, reported and zeroed) Just zeroed does not exist at this
> stage in the series.
>

More of a forward looking question: if we already know we're adding
was_reported and was_zeroed, maybe we should just pass the page flags
through entirely and let expand() check them instead of passing it
through individually.

We end up doing something similar with post_alloc_hook later as well, so
I'm wondering if there's a general improvement that can be made here.

~Gregory

^ permalink raw reply

* Re: [PATCH v7 02/31] mm: page_alloc: propagate PageReported flag across buddy splits
From: Michael S. Tsirkin @ 2026-05-14 15:47 UTC (permalink / raw)
  To: Gregory Price
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <agXtOeuYNin9nYN8@gourry-fedora-PF4VCD3F>

On Thu, May 14, 2026 at 11:41:45AM -0400, Gregory Price wrote:
> On Thu, May 14, 2026 at 11:34:14AM -0400, Michael S. Tsirkin wrote:
> > On Thu, May 14, 2026 at 11:32:26AM -0400, Gregory Price wrote:
> > > > > 
> > > > > Maybe mildly out of scope but worth asking:  Are there other flags that
> > > > > should be retained/propogated on a split?  If so, rather than pass
> > > > > was_reported, should we just take a temporary copy of the page flags and
> > > > > pass them all in?
> > > > > 
> > > > > ~Gregory
> > > > 
> > > > 
> > > > Not that I can see, no.
> > > > 
> > > 
> > > You do this exact thing again later :]
> > > 
> > > https://lore.kernel.org/linux-mm/9a22e0f9bbe1278913754db6df76e291a006181a.1778616612.git.mst@redhat.com/
> > > 
> > > ~Gregory
> > 
> > I mean yes, reported and zeroed) Just zeroed does not exist at this
> > stage in the series.
> >
> 
> More of a forward looking question: if we already know we're adding
> was_reported and was_zeroed, maybe we should just pass the page flags
> through entirely and let expand() check them instead of passing it
> through individually.
> 
> We end up doing something similar with post_alloc_hook later as well, so
> I'm wondering if there's a general improvement that can be made here.
> 
> ~Gregory

sure, will do. do you want a bitwise thing for type safety? or just a
long?


^ permalink raw reply

* Re: [PATCH v6 5/7] locking: Add contended_release tracepoint to qspinlock
From: Steven Rostedt @ 2026-05-14 16:03 UTC (permalink / raw)
  To: Dmitry Ilvokhin
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
	Thomas Bogendoerfer, Juergen Gross, Ajay Kaher, Alexey Makhalov,
	Broadcom internal kernel review list, Thomas Gleixner,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Arnd Bergmann,
	Dennis Zhou, Tejun Heo, Christoph Lameter, Masami Hiramatsu,
	Mathieu Desnoyers, linux-kernel, linux-mips, virtualization,
	linux-arch, linux-mm, linux-trace-kernel, kernel-team,
	Paul E. McKenney
In-Reply-To: <agXYjw3GM15WtC-H@shell.ilvokhin.com>

On Thu, 14 May 2026 14:13:35 +0000
Dmitry Ilvokhin <d@ilvokhin.com> wrote:

> > > +void __lockfunc queued_spin_release_traced(struct qspinlock *lock)
> > > +{
> > > +	if (queued_spin_is_contended(lock))
> > > +		trace_call__contended_release(lock);
> > > +	queued_spin_release(lock);  
> > 
> > And then remove the duplicate call of "queued_spin_release()" here.  
> 
> This is the scenario the comment above the static branch describes.
> Here's what it looks like in practice on x86_64 (defconfig, compiled
> with GCC 11).
> 
> Current design (trace + unlock combined, with return):
>   
>     endbr64
>     xchg %ax,%ax                     ; NOP (static branch)
>     movb $0x0,(%rdi)                 ; unlock
>     decl %gs:__preempt_count
>     je   preempt
>     jmp  __x86_return_thunk
>     call queued_spin_release_traced  ; cold
>     jmp  preempt_handling            ; cold
>     call __SCT__preempt_schedule
>     jmp  __x86_return_thunk
> 
> With the trace-only function (no return, unlock after the call):
>   
>     endbr64
>     push %rbx                        ; saves callee-saved rbx (!)
>     mov  %rdi,%rbx                   ; preserve lock across call (!)
>     xchg %ax,%ax                     ; NOP (static branch)
>     movb $0x0,(%rbx)                 ; unlock
>     decl %gs:__preempt_count
>     je   preempt
>     pop  %rbx                        ; callee-saved restore (!)
>     jmp  __x86_return_thunk
>     call queued_spin_release_traced  ; cold
>     jmp  unlock                      ; cold
>     call __SCT__preempt_schedule
>     pop  %rbx
>     jmp  __x86_return_thunk
> 
> Three extra instructions marked by "!" on the hot path (push, mov, pop),
> all wasted when the tracepoint is off. That's the main reason for
> combining trace and unlock in the same out-of-line function.

Ah, because the return makes it into two tail calls.

I still don't like the duplication, perhaps add some more comments about
needing to update the other location if anything changes here? And perhaps
comment that this duplicate code helps the assembly.

-- Steve


^ permalink raw reply

* Re: [PATCH v5 0/3] vfio/pci: Request resources and map BARs at enable time
From: Alex Williamson @ 2026-05-14 16:38 UTC (permalink / raw)
  To: Matt Evans
  Cc: Kevin Tian, Jason Gunthorpe, Ankit Agrawal, Alistair Popple,
	Leon Romanovsky, Kees Cook, Shameer Kolothum, Yishai Hadas,
	Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy,
	Zhi Wang, kvm, linux-kernel, virtualization, alex
In-Reply-To: <20260511145829.2993601-1-mattev@meta.com>

On Mon, 11 May 2026 07:58:22 -0700
Matt Evans <mattev@meta.com> wrote:

> Hi,
> 
> These patches fix a potential race for concurrent calls to
> vfio_pci_core_setup_barmap(), and a DMABUF missing check for resource
> before the export.  Discussion on a previous series (different,
> replaced by this one) is here:
> 
>  https://lore.kernel.org/kvm/20260415181423.1008458-1-mattev@meta.com
> 
> Responses in that thread indicated there wasn't a strong historical
> reason to require the mapping to be performed on-demand at BAR
> reference time.  It's much simpler to move this earlier, to
> vfio_pci_core_enable(), and that then avoids having to deal with
> concurrent requests later.
> 
> The first patch requests PCI resources and pci_iomap() of the BARs
> from vfio_pci_core_enable(), moving this out of
> vfio_pci_core_setup_barmap().
> 
> Some callers rely on vfio_pci_core_setup_barmap() for its ioremap()
> effect, and other callers use it for its resource-acquiring effect.
> The function turns into a cheap error check that both these actions
> have occurred; that maintains the same error behaviour as before the
> fix.
> 
> The second patch adds a call to vfio_pci_core_setup_barmap() to VFIO
> DMABUF export to check the resource is reserved; previously this was
> able to export an unrequested resource.  Although patch 1 at first
> appears to fix this by requesting resources at enable time, code using
> the BAR still needs to check the resource really was acquired.
> 
> (FWIW, Leon gave a R-B on v2 in
> https://lore.kernel.org/kvm/20260426111606.GC440345@unreal/ though
> this repost isn't 100% identical so I didn't propagate it.)
> 
> The third patch refactors vfio_pci_core_setup_barmap() plus the various
> vdev->barmap[] accesses into vfio_pci_core_get_iomap() which returns
> either a pointer to the mapping or an ERR_PTR() describing why it
> doesn't exist.  This is used by callers that need the mapping, but
> also by other callers to check that the resource/mapping step was
> successful.
> 
> Matt Evans (3):
>   vfio/pci: Set up BAR resources and maps in vfio_pci_core_enable()
>   vfio/pci: Check BAR resources before exporting a DMABUF
>   vfio/pci: Replace vfio_pci_core_setup_barmap() with
>     vfio_pci_core_get_iomap()
> 
>  drivers/vfio/pci/nvgrace-gpu/main.c | 20 +++++++-----
>  drivers/vfio/pci/vfio_pci_core.c    | 48 ++++++++++++++++++++++++-----
>  drivers/vfio/pci/vfio_pci_dmabuf.c  |  6 ++--
>  drivers/vfio/pci/vfio_pci_rdwr.c    | 42 +++++--------------------
>  drivers/vfio/pci/virtio/legacy_io.c | 13 ++++----
>  include/linux/vfio_pci_core.h       | 20 +++++++++++-
>  6 files changed, 91 insertions(+), 58 deletions(-)
> 

Applied 1/ and 2/ to the vfio for-linus branch for v7.1-rc.  My
intention is to have these merged for rc4, which I'll then use as the
base for the v7.2 queue where we can pull in 3/.  Thanks,

Alex

^ permalink raw reply

* Re: [PATCH net v3 1/2] vsock/virtio: reset connection on receiving queue overflow
From: Stefano Garzarella @ 2026-05-14 16:45 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, Xuan Zhuo, Eugenio Pérez, linux-kernel, Simon Horman,
	Paolo Abeni, Jakub Kicinski, Jason Wang, kvm, Stefan Hajnoczi,
	virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <20260514111513-mutt-send-email-mst@kernel.org>

On Thu, 14 May 2026 at 17:16, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, May 14, 2026 at 04:57:16PM +0200, Stefano Garzarella wrote:
> > On Wed, May 13, 2026 at 12:54:16PM +0200, Stefano Garzarella wrote:
> > > From: Stefano Garzarella <sgarzare@redhat.com>
> > >
> > > When there is no more space to queue an incoming packet, the packet is
> > > silently dropped. This causes data loss without any notification to
> > > either peer, since there is no retransmission.
> > >
> > > Under normal circumstances, this should never happen. However, it could
> > > happen if the other peer doesn't respect the credit, or if the skb
> > > overhead, which we recently began to take into account with commit
> > > 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue"),
> > > is too high.
> > >
> > > Fix this by resetting the connection and setting the local socket error
> > > to ENOBUFS when virtio_transport_recv_enqueue() can no longer queue a
> > > packet, so both peers are explicitly notified of the failure rather than
> > > silently losing data.
> > >
> > > Fixes: ae6fcfbf5f03 ("vsock/virtio: discard packets if credit is not respected")
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > > net/vmw_vsock/virtio_transport_common.c | 19 ++++++++++++++-----
> > > 1 file changed, 14 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > > index 989cc252d3d3..4a4ac69d1ad1 100644
> > > --- a/net/vmw_vsock/virtio_transport_common.c
> > > +++ b/net/vmw_vsock/virtio_transport_common.c
> > > @@ -1350,7 +1350,7 @@ virtio_transport_recv_connecting(struct sock *sk,
> > >     return err;
> > > }
> > >
> > > -static void
> > > +static bool
> > > virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > >                           struct sk_buff *skb)
> > > {
> > > @@ -1365,10 +1365,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > >     spin_lock_bh(&vvs->rx_lock);
> > >
> > >     can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
> > > -   if (!can_enqueue) {
> > > -           free_pkt = true;
> > > +   if (!can_enqueue)
> > >             goto out;
> > > -   }
> > >
> > >     if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
> > >             vvs->msg_count++;
> > > @@ -1408,6 +1406,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > >     spin_unlock_bh(&vvs->rx_lock);
> > >     if (free_pkt)
> > >             kfree_skb(skb);
> > > +
> > > +   return can_enqueue;
> > > }
> > >
> > > static int
> > > @@ -1420,7 +1420,16 @@ virtio_transport_recv_connected(struct sock *sk,
> > >
> > >     switch (le16_to_cpu(hdr->op)) {
> > >     case VIRTIO_VSOCK_OP_RW:
> > > -           virtio_transport_recv_enqueue(vsk, skb);
> > > +           if (!virtio_transport_recv_enqueue(vsk, skb)) {
> > > +                   /* There is no more space to queue the packet, so let's
> > > +                    * close the connection; otherwise, we'll lose data.
> > > +                    */
> > > +                   (void)virtio_transport_reset(vsk, skb);
> > > +                   sk->sk_state = TCP_CLOSE;
> > > +                   sk->sk_err = ENOBUFS;
> > > +                   sk_error_report(sk);
> >
> > sashiko reported some issues related to setting TCP_CLOSE state and not
> > removing the socket from the connect table:
> > https://sashiko.dev/#/patchset/20260513105417.56761-1-sgarzare%40redhat.com
> >
> > I'll change this by calling virtio_transport_do_close() and
> > vsock_remove_sock() in the next version.
> >
> > Stefano
> >
> > > +                   break;
> > > +           }
> > >             vsock_data_ready(sk);
> > >             return err;
> > >     case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
> > > --
> > > 2.54.0
> > >
>
>
> And so the bag of hacks grows. I feel this is energy not well spent.
> Please, let us fix this properly *first*. And then worry about how to
> backport.  Maybe it will not be so terrible to backport after all.
>

TBH I don't think this is an hack, but an issue we should fix in any case.
Regarding the second patch, I see your point, but it's a big change
that worries me. I'd like some more time to fix it properly without
rushing. Staying calm without realizing that userspace is broken like
we are now without this series :-(

That said, evaluating further, I think we have a similar issue also
with STREAM on the host side where the skb usually doesn't free space,
so we need a merge strategy also there.

So, I'd like to have time to fix both definitely. If you have time and
want to go ahead, please do.

Thanks,
Stefano


^ permalink raw reply

* Re: [PATCH net v3 1/2] vsock/virtio: reset connection on receiving queue overflow
From: Michael S. Tsirkin @ 2026-05-14 17:44 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: netdev, Xuan Zhuo, Eugenio Pérez, linux-kernel, Simon Horman,
	Paolo Abeni, Jakub Kicinski, Jason Wang, kvm, Stefan Hajnoczi,
	virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <CAGxU2F4qWcQTmXnH74V8xZKADLiEMUmCc0+ktW5fZOXdTP4Bdw@mail.gmail.com>

On Thu, May 14, 2026 at 06:45:00PM +0200, Stefano Garzarella wrote:
> On Thu, 14 May 2026 at 17:16, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, May 14, 2026 at 04:57:16PM +0200, Stefano Garzarella wrote:
> > > On Wed, May 13, 2026 at 12:54:16PM +0200, Stefano Garzarella wrote:
> > > > From: Stefano Garzarella <sgarzare@redhat.com>
> > > >
> > > > When there is no more space to queue an incoming packet, the packet is
> > > > silently dropped. This causes data loss without any notification to
> > > > either peer, since there is no retransmission.
> > > >
> > > > Under normal circumstances, this should never happen. However, it could
> > > > happen if the other peer doesn't respect the credit, or if the skb
> > > > overhead, which we recently began to take into account with commit
> > > > 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue"),
> > > > is too high.
> > > >
> > > > Fix this by resetting the connection and setting the local socket error
> > > > to ENOBUFS when virtio_transport_recv_enqueue() can no longer queue a
> > > > packet, so both peers are explicitly notified of the failure rather than
> > > > silently losing data.
> > > >
> > > > Fixes: ae6fcfbf5f03 ("vsock/virtio: discard packets if credit is not respected")
> > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > > ---
> > > > net/vmw_vsock/virtio_transport_common.c | 19 ++++++++++++++-----
> > > > 1 file changed, 14 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > > > index 989cc252d3d3..4a4ac69d1ad1 100644
> > > > --- a/net/vmw_vsock/virtio_transport_common.c
> > > > +++ b/net/vmw_vsock/virtio_transport_common.c
> > > > @@ -1350,7 +1350,7 @@ virtio_transport_recv_connecting(struct sock *sk,
> > > >     return err;
> > > > }
> > > >
> > > > -static void
> > > > +static bool
> > > > virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > > >                           struct sk_buff *skb)
> > > > {
> > > > @@ -1365,10 +1365,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > > >     spin_lock_bh(&vvs->rx_lock);
> > > >
> > > >     can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
> > > > -   if (!can_enqueue) {
> > > > -           free_pkt = true;
> > > > +   if (!can_enqueue)
> > > >             goto out;
> > > > -   }
> > > >
> > > >     if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
> > > >             vvs->msg_count++;
> > > > @@ -1408,6 +1406,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > > >     spin_unlock_bh(&vvs->rx_lock);
> > > >     if (free_pkt)
> > > >             kfree_skb(skb);
> > > > +
> > > > +   return can_enqueue;
> > > > }
> > > >
> > > > static int
> > > > @@ -1420,7 +1420,16 @@ virtio_transport_recv_connected(struct sock *sk,
> > > >
> > > >     switch (le16_to_cpu(hdr->op)) {
> > > >     case VIRTIO_VSOCK_OP_RW:
> > > > -           virtio_transport_recv_enqueue(vsk, skb);
> > > > +           if (!virtio_transport_recv_enqueue(vsk, skb)) {
> > > > +                   /* There is no more space to queue the packet, so let's
> > > > +                    * close the connection; otherwise, we'll lose data.
> > > > +                    */
> > > > +                   (void)virtio_transport_reset(vsk, skb);
> > > > +                   sk->sk_state = TCP_CLOSE;
> > > > +                   sk->sk_err = ENOBUFS;
> > > > +                   sk_error_report(sk);
> > >
> > > sashiko reported some issues related to setting TCP_CLOSE state and not
> > > removing the socket from the connect table:
> > > https://sashiko.dev/#/patchset/20260513105417.56761-1-sgarzare%40redhat.com
> > >
> > > I'll change this by calling virtio_transport_do_close() and
> > > vsock_remove_sock() in the next version.
> > >
> > > Stefano
> > >
> > > > +                   break;
> > > > +           }
> > > >             vsock_data_ready(sk);
> > > >             return err;
> > > >     case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
> > > > --
> > > > 2.54.0
> > > >
> >
> >
> > And so the bag of hacks grows. I feel this is energy not well spent.
> > Please, let us fix this properly *first*. And then worry about how to
> > backport.  Maybe it will not be so terrible to backport after all.
> >
> 
> TBH I don't think this is an hack, but an issue we should fix in any case.
> Regarding the second patch, I see your point, but it's a big change
> that worries me. I'd like some more time to fix it properly without
> rushing. Staying calm without realizing that userspace is broken like
> we are now without this series :-(
> 
> That said, evaluating further, I think we have a similar issue also
> with STREAM on the host side where the skb usually doesn't free space,
> so we need a merge strategy also there.
> 
> So, I'd like to have time to fix both definitely. If you have time and
> want to go ahead, please do.
> 
> Thanks,
> Stefano

Well my patch was a start, we just need a strategy how to avoid copying
everything, right?

-- 
MST


^ permalink raw reply

* Re: [PATCH v7 02/31] mm: page_alloc: propagate PageReported flag across buddy splits
From: Gregory Price @ 2026-05-14 17:48 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <20260514114637-mutt-send-email-mst@kernel.org>

On Thu, May 14, 2026 at 11:47:19AM -0400, Michael S. Tsirkin wrote:
> On Thu, May 14, 2026 at 11:41:45AM -0400, Gregory Price wrote:
> > On Thu, May 14, 2026 at 11:34:14AM -0400, Michael S. Tsirkin wrote:
> > > On Thu, May 14, 2026 at 11:32:26AM -0400, Gregory Price wrote:
> > > > > > 
> > > > > > Maybe mildly out of scope but worth asking:  Are there other flags that
> > > > > > should be retained/propogated on a split?  If so, rather than pass
> > > > > > was_reported, should we just take a temporary copy of the page flags and
> > > > > > pass them all in?
> > > > > > 
> > > > > > ~Gregory
> > > > > 
> > > > > 
> > > > > Not that I can see, no.
> > > > > 
> > > > 
> > > > You do this exact thing again later :]
> > > > 
> > > > https://lore.kernel.org/linux-mm/9a22e0f9bbe1278913754db6df76e291a006181a.1778616612.git.mst@redhat.com/
> > > > 
> > > > ~Gregory
> > > 
> > > I mean yes, reported and zeroed) Just zeroed does not exist at this
> > > stage in the series.
> > >
> > 
> > More of a forward looking question: if we already know we're adding
> > was_reported and was_zeroed, maybe we should just pass the page flags
> > through entirely and let expand() check them instead of passing it
> > through individually.
> > 
> > We end up doing something similar with post_alloc_hook later as well, so
> > I'm wondering if there's a general improvement that can be made here.
> > 
> > ~Gregory
> 
> sure, will do. do you want a bitwise thing for type safety? or just a
> long?
>

Can probably just do like:

flags = stable_page_flags(page);

plumbing...

expand(..., buddy_flags) {
	if (test_bit(PG_reported, buddy_flags))
		...
	if (test_bit(PG_zeroed, buddy_flags))
		...
}

or whatever makes sense.

~Gregory

^ permalink raw reply

* Re: [PATCH v7 02/31] mm: page_alloc: propagate PageReported flag across buddy splits
From: Michael S. Tsirkin @ 2026-05-14 17:51 UTC (permalink / raw)
  To: Gregory Price
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <agYK9JKFInSwKSZm@gourry-fedora-PF4VCD3F>

On Thu, May 14, 2026 at 01:48:36PM -0400, Gregory Price wrote:
> On Thu, May 14, 2026 at 11:47:19AM -0400, Michael S. Tsirkin wrote:
> > On Thu, May 14, 2026 at 11:41:45AM -0400, Gregory Price wrote:
> > > On Thu, May 14, 2026 at 11:34:14AM -0400, Michael S. Tsirkin wrote:
> > > > On Thu, May 14, 2026 at 11:32:26AM -0400, Gregory Price wrote:
> > > > > > > 
> > > > > > > Maybe mildly out of scope but worth asking:  Are there other flags that
> > > > > > > should be retained/propogated on a split?  If so, rather than pass
> > > > > > > was_reported, should we just take a temporary copy of the page flags and
> > > > > > > pass them all in?
> > > > > > > 
> > > > > > > ~Gregory
> > > > > > 
> > > > > > 
> > > > > > Not that I can see, no.
> > > > > > 
> > > > > 
> > > > > You do this exact thing again later :]
> > > > > 
> > > > > https://lore.kernel.org/linux-mm/9a22e0f9bbe1278913754db6df76e291a006181a.1778616612.git.mst@redhat.com/
> > > > > 
> > > > > ~Gregory
> > > > 
> > > > I mean yes, reported and zeroed) Just zeroed does not exist at this
> > > > stage in the series.
> > > >
> > > 
> > > More of a forward looking question: if we already know we're adding
> > > was_reported and was_zeroed, maybe we should just pass the page flags
> > > through entirely and let expand() check them instead of passing it
> > > through individually.
> > > 
> > > We end up doing something similar with post_alloc_hook later as well, so
> > > I'm wondering if there's a general improvement that can be made here.
> > > 
> > > ~Gregory
> > 
> > sure, will do. do you want a bitwise thing for type safety? or just a
> > long?
> >
> 
> Can probably just do like:
> 
> flags = stable_page_flags(page);
> 
> plumbing...
> 
> expand(..., buddy_flags) {
> 	if (test_bit(PG_reported, buddy_flags))
> 		...
> 	if (test_bit(PG_zeroed, buddy_flags))
> 		...
> }
> 
> or whatever makes sense.
> 
> ~Gregory

right, that's clear.


^ permalink raw reply

* Re: [PATCH v7 09/31] mm: use folio_zero_user for user pages in post_alloc_hook
From: Michael S. Tsirkin @ 2026-05-14 18:00 UTC (permalink / raw)
  To: Gregory Price
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <agXS7dMmtJrPhu9W@gourry-fedora-PF4VCD3F>

On Thu, May 14, 2026 at 09:49:33AM -0400, Gregory Price wrote:
> On Tue, May 12, 2026 at 05:05:54PM -0400, Michael S. Tsirkin wrote:
> > When post_alloc_hook() needs to zero a page for an explicit
> > __GFP_ZERO allocation for a user page (user_addr is set), use folio_zero_user()
> > instead of kernel_init_pages().  This zeros near the faulting
> > address last, keeping those cachelines hot for the impending
> > user access.
> > 
> > folio_zero_user() is only used for explicit __GFP_ZERO, not for
> > init_on_alloc.  On architectures with virtually-indexed caches
> > (e.g., ARM), clear_user_highpage() performs per-line cache
> > operations; using it for init_on_alloc would add overhead that
> > kernel_init_pages() avoids (the page fault path flushes the
> > cache at PTE installation time regardless).
> > 
> > No functional change yet: current callers do not pass __GFP_ZERO
> > for user pages (they zero at the callsite instead).  Subsequent
> > patches will convert them.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Assisted-by: Claude:claude-opus-4-6
> > ---
> >  mm/page_alloc.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index db387dd6b813..76f39dd026ff 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1861,9 +1861,20 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
> >  		for (i = 0; i != 1 << order; ++i)
> >  			page_kasan_tag_reset(page + i);
> >  	}
> > -	/* If memory is still not initialized, initialize it now. */
> > -	if (init)
> > -		kernel_init_pages(page, 1 << order);
> > +	/*
> > +	 * If memory is still not initialized, initialize it now.
> > +	 * When __GFP_ZERO was explicitly requested and user_addr is set,
> > +	 * use folio_zero_user() which zeros near the faulting address
> > +	 * last, keeping those cachelines hot.  For init_on_alloc, use
> > +	 * kernel_init_pages() to avoid unnecessary cache flush overhead
> > +	 * on architectures with virtually-indexed caches.
> > +	 */
> > +	if (init) {
> > +		if ((gfp_flags & __GFP_ZERO) && user_addr != USER_ADDR_NONE)
> > +			folio_zero_user(page_folio(page), user_addr);
> > +		else
> > +			kernel_init_pages(page, 1 << order);
> > +	}
> 
> Open question but not necessarily in-scope:
> 
> Should __GFP_ZERO just be implied if (user_addr != USER_ADDR_NONE)?


There are calls with no __GFP_ZERO but they do not allocate userspace pages.

  - drm_pagemap.c: GFP_HIGHUSER -- no zero. But this is a DRM device
    page migration, the page content is preserved from the source.

  - test_hmm.c: GFP_HIGHUSER_MOVABLE -- no zero. Test driver, pages get
    content from device.

  - mm/ksm.c: GFP_HIGHUSER_MOVABLE -- no zero. KSM merges identical
    pages, content comes from the source page (copy).

  - mm/memory.c new_folio = GFP_HIGHUSER_MOVABLE
    - no zero. This is CoW, content is copied from old page.

  - mm/userfaultfd.c: GFP_HIGHUSER_MOVABLE - no zero. Content comes from userspace via userfaultfd.

  - arm64/fault.c: __GFP_ZEROTAGS not __GFP_ZERO. MTE tag zeroing, not page zeroing. Page is zeroed separately.


> Putting aside how that's done without introducing another gfp flag
> (maybe something explicit like `alloc_pages_nozero(...)` ), it seems
> like a very short jump to just adding __GFP_ZERO to any user-alloc by
> default.
> 
> I'd be curious to know how many callers across the system omit
> __GFP_ZERO when allocating a user-page, and whether there might be
> scenarios where we subtly miss it (seems unlikely and narrow, but very
> possibly something a driver could do unintentionally).
> 
> ~Gregory


I'd do this on top if possible.

-- 
MST


^ permalink raw reply

* Re: [PATCH v7 09/31] mm: use folio_zero_user for user pages in post_alloc_hook
From: Gregory Price @ 2026-05-14 18:56 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <20260514135214-mutt-send-email-mst@kernel.org>

On Thu, May 14, 2026 at 02:00:31PM -0400, Michael S. Tsirkin wrote:
> On Thu, May 14, 2026 at 09:49:33AM -0400, Gregory Price wrote:
> 
> There are calls with no __GFP_ZERO but they do not allocate userspace pages.
> 
>   - drm_pagemap.c: GFP_HIGHUSER -- no zero. But this is a DRM device
>     page migration, the page content is preserved from the source.
> 
>   - test_hmm.c: GFP_HIGHUSER_MOVABLE -- no zero. Test driver, pages get
>     content from device.
> 
>   - mm/ksm.c: GFP_HIGHUSER_MOVABLE -- no zero. KSM merges identical
>     pages, content comes from the source page (copy).
> 
>   - mm/memory.c new_folio = GFP_HIGHUSER_MOVABLE
>     - no zero. This is CoW, content is copied from old page.
> 
>   - mm/userfaultfd.c: GFP_HIGHUSER_MOVABLE - no zero. Content comes from userspace via userfaultfd.
> 
>   - arm64/fault.c: __GFP_ZEROTAGS not __GFP_ZERO. MTE tag zeroing, not page zeroing. Page is zeroed separately.
> 

Right, so in all of these cases, it would be just as correct to pass
USER_ADDR_NONE I imagine :]

i.e. the user address is irrelevant, and the caller is responsible for
     sanitization before return if it's relevant.

Otherwise, passing (user_addr != -1) the buddy takes care of it for you.

Just an obvious security bonus to all of this, but by no means a
requirement for your set.  Just an observation.

> 
> I'd do this on top if possible.
>

Yeah reasonable.

~Gregory

^ permalink raw reply

* Re: [PATCH v7 09/31] mm: use folio_zero_user for user pages in post_alloc_hook
From: Michael S. Tsirkin @ 2026-05-14 19:08 UTC (permalink / raw)
  To: Gregory Price
  Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
	Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
	Alistair Popple, Christoph Lameter, David Rientjes,
	Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
	virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <agYa9s5B7OWG-kjR@gourry-fedora-PF4VCD3F>

On Thu, May 14, 2026 at 02:56:54PM -0400, Gregory Price wrote:
> On Thu, May 14, 2026 at 02:00:31PM -0400, Michael S. Tsirkin wrote:
> > On Thu, May 14, 2026 at 09:49:33AM -0400, Gregory Price wrote:
> > 
> > There are calls with no __GFP_ZERO but they do not allocate userspace pages.
> > 
> >   - drm_pagemap.c: GFP_HIGHUSER -- no zero. But this is a DRM device
> >     page migration, the page content is preserved from the source.
> > 
> >   - test_hmm.c: GFP_HIGHUSER_MOVABLE -- no zero. Test driver, pages get
> >     content from device.
> > 
> >   - mm/ksm.c: GFP_HIGHUSER_MOVABLE -- no zero. KSM merges identical
> >     pages, content comes from the source page (copy).
> > 
> >   - mm/memory.c new_folio = GFP_HIGHUSER_MOVABLE
> >     - no zero. This is CoW, content is copied from old page.
> > 
> >   - mm/userfaultfd.c: GFP_HIGHUSER_MOVABLE - no zero. Content comes from userspace via userfaultfd.
> > 
> >   - arm64/fault.c: __GFP_ZEROTAGS not __GFP_ZERO. MTE tag zeroing, not page zeroing. Page is zeroed separately.
> > 
> 
> Right, so in all of these cases, it would be just as correct to pass
> USER_ADDR_NONE I imagine :]

Hmm. Are you sure? Isn't the address used for numa policy?

> i.e. the user address is irrelevant, and the caller is responsible for
>      sanitization before return if it's relevant.
> 
> Otherwise, passing (user_addr != -1) the buddy takes care of it for you.
> 
> Just an obvious security bonus to all of this, but by no means a
> requirement for your set.  Just an observation.
> 
> > 
> > I'd do this on top if possible.
> >
> 
> Yeah reasonable.
> 
> ~Gregory


^ permalink raw reply

* Re: [PATCH net v3 1/2] vsock/virtio: reset connection on receiving queue overflow
From: Stefano Garzarella @ 2026-05-15  8:29 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, Xuan Zhuo, Eugenio Pérez, linux-kernel, Simon Horman,
	Paolo Abeni, Jakub Kicinski, Jason Wang, kvm, Stefan Hajnoczi,
	virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <20260514134347-mutt-send-email-mst@kernel.org>

On Thu, May 14, 2026 at 01:44:53PM -0400, Michael S. Tsirkin wrote:
>On Thu, May 14, 2026 at 06:45:00PM +0200, Stefano Garzarella wrote:
>> On Thu, 14 May 2026 at 17:16, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >
>> > On Thu, May 14, 2026 at 04:57:16PM +0200, Stefano Garzarella wrote:
>> > > On Wed, May 13, 2026 at 12:54:16PM +0200, Stefano Garzarella wrote:
>> > > > From: Stefano Garzarella <sgarzare@redhat.com>
>> > > >
>> > > > When there is no more space to queue an incoming packet, the packet is
>> > > > silently dropped. This causes data loss without any notification to
>> > > > either peer, since there is no retransmission.
>> > > >
>> > > > Under normal circumstances, this should never happen. However, it could
>> > > > happen if the other peer doesn't respect the credit, or if the skb
>> > > > overhead, which we recently began to take into account with commit
>> > > > 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue"),
>> > > > is too high.
>> > > >
>> > > > Fix this by resetting the connection and setting the local socket error
>> > > > to ENOBUFS when virtio_transport_recv_enqueue() can no longer queue a
>> > > > packet, so both peers are explicitly notified of the failure rather than
>> > > > silently losing data.
>> > > >
>> > > > Fixes: ae6fcfbf5f03 ("vsock/virtio: discard packets if credit is not respected")
>> > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> > > > ---
>> > > > net/vmw_vsock/virtio_transport_common.c | 19 ++++++++++++++-----
>> > > > 1 file changed, 14 insertions(+), 5 deletions(-)
>> > > >
>> > > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> > > > index 989cc252d3d3..4a4ac69d1ad1 100644
>> > > > --- a/net/vmw_vsock/virtio_transport_common.c
>> > > > +++ b/net/vmw_vsock/virtio_transport_common.c
>> > > > @@ -1350,7 +1350,7 @@ virtio_transport_recv_connecting(struct sock *sk,
>> > > >     return err;
>> > > > }
>> > > >
>> > > > -static void
>> > > > +static bool
>> > > > virtio_transport_recv_enqueue(struct vsock_sock *vsk,
>> > > >                           struct sk_buff *skb)
>> > > > {
>> > > > @@ -1365,10 +1365,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
>> > > >     spin_lock_bh(&vvs->rx_lock);
>> > > >
>> > > >     can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
>> > > > -   if (!can_enqueue) {
>> > > > -           free_pkt = true;
>> > > > +   if (!can_enqueue)
>> > > >             goto out;
>> > > > -   }
>> > > >
>> > > >     if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
>> > > >             vvs->msg_count++;
>> > > > @@ -1408,6 +1406,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
>> > > >     spin_unlock_bh(&vvs->rx_lock);
>> > > >     if (free_pkt)
>> > > >             kfree_skb(skb);
>> > > > +
>> > > > +   return can_enqueue;
>> > > > }
>> > > >
>> > > > static int
>> > > > @@ -1420,7 +1420,16 @@ virtio_transport_recv_connected(struct sock *sk,
>> > > >
>> > > >     switch (le16_to_cpu(hdr->op)) {
>> > > >     case VIRTIO_VSOCK_OP_RW:
>> > > > -           virtio_transport_recv_enqueue(vsk, skb);
>> > > > +           if (!virtio_transport_recv_enqueue(vsk, skb)) {
>> > > > +                   /* There is no more space to queue the packet, so let's
>> > > > +                    * close the connection; otherwise, we'll lose data.
>> > > > +                    */
>> > > > +                   (void)virtio_transport_reset(vsk, skb);
>> > > > +                   sk->sk_state = TCP_CLOSE;
>> > > > +                   sk->sk_err = ENOBUFS;
>> > > > +                   sk_error_report(sk);
>> > >
>> > > sashiko reported some issues related to setting TCP_CLOSE state and not
>> > > removing the socket from the connect table:
>> > > https://sashiko.dev/#/patchset/20260513105417.56761-1-sgarzare%40redhat.com
>> > >
>> > > I'll change this by calling virtio_transport_do_close() and
>> > > vsock_remove_sock() in the next version.
>> > >
>> > > Stefano
>> > >
>> > > > +                   break;
>> > > > +           }
>> > > >             vsock_data_ready(sk);
>> > > >             return err;
>> > > >     case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
>> > > > --
>> > > > 2.54.0
>> > > >
>> >
>> >
>> > And so the bag of hacks grows. I feel this is energy not well spent.
>> > Please, let us fix this properly *first*. And then worry about how to
>> > backport.  Maybe it will not be so terrible to backport after all.
>> >
>>
>> TBH I don't think this is an hack, but an issue we should fix in any case.
>> Regarding the second patch, I see your point, but it's a big change
>> that worries me. I'd like some more time to fix it properly without
>> rushing. Staying calm without realizing that userspace is broken like
>> we are now without this series :-(
>>
>> That said, evaluating further, I think we have a similar issue also
>> with STREAM on the host side where the skb usually doesn't free space,
>> so we need a merge strategy also there.
>>
>> So, I'd like to have time to fix both definitely. If you have time and
>> want to go ahead, please do.
>>
>> Thanks,
>> Stefano
>
>Well my patch was a start, we just need a strategy how to avoid copying
>everything, right?

Yep, and then there's the question of how to handle EOM without a 
payload, but I think that's a special case. In theory, we don't support 
sending it, but I'm not sure if POSIX allows it or not.

That said, is it okay if I send a v4 of this series?

(I'm not sure if I'll be able to work on the merging next week)

Stefano


^ permalink raw reply

* [PATCH v3] drm/virtio: use uninterruptible resv lock for plane updates
From: Deepanshu Kartikey @ 2026-05-15  8:40 UTC (permalink / raw)
  To: airlied, kraxel, dmitry.osipenko, gurchetansingh, olvaffe,
	maarten.lankhorst, mripard, tzimmermann, simona, sumit.semwal,
	christian.koenig
  Cc: dri-devel, virtualization, linux-kernel, linux-media,
	linaro-mm-sig, Deepanshu Kartikey, syzbot+72bd3dd3a5d5f39a0271,
	stable

virtio_gpu_cursor_plane_update() and virtio_gpu_resource_flush() lock
the framebuffer BO's dma_resv via virtio_gpu_array_lock_resv() and
ignore its return value. The function can fail with -EINTR from
dma_resv_lock_interruptible() (signal during lock wait) or with
-ENOMEM from dma_resv_reserve_fences() (fence slot allocation),
leaving the resv lock not held. The queue path then walks the object
array and calls dma_resv_add_fence(), which requires the lock held;
with lockdep enabled this trips dma_resv_assert_held():

  WARNING: drivers/dma-buf/dma-resv.c:296 at dma_resv_add_fence+0x71e/0x840
  Call Trace:
   virtio_gpu_array_add_fence
   virtio_gpu_queue_ctrl_sgs
   virtio_gpu_queue_fenced_ctrl_buffer
   virtio_gpu_cursor_plane_update
   drm_atomic_helper_commit_planes
   drm_atomic_helper_commit_tail
   commit_tail
   drm_atomic_helper_commit
   drm_atomic_commit
   drm_atomic_helper_update_plane
   __setplane_atomic
   drm_mode_cursor_universal
   drm_mode_cursor_common
   drm_mode_cursor_ioctl
   drm_ioctl
   __x64_sys_ioctl

Beyond the WARN, mutating the dma_resv fence list without the lock
races with concurrent readers/writers and can corrupt the list.

Both call sites run inside the .atomic_update plane callback, which
DRM atomic helpers do not allow to fail (by the time it runs, the
commit has been signed off to userspace and there is no clean
rollback path). Moving the lock acquisition to .prepare_fb (v2) was
rejected because the broader lock scope deadlocks against other
BO locking paths in the same atomic commit.

Introduce virtio_gpu_array_lock_resv_uninterruptible() that uses
dma_resv_lock() instead of dma_resv_lock_interruptible() on the
nents==1 path. This eliminates the -EINTR failure mode -- the
realistic syzbot trigger -- without extending the lock hold across
the commit. Use it from both virtio_gpu_cursor_plane_update() and
virtio_gpu_resource_flush(); check the return value to handle the
remaining -ENOMEM case by freeing the objs and skipping the plane
update for that frame. The framebuffer BOs touched here are not
shared with other contexts and lock contention is expected to be
brief, so the loss of signal-interruptibility is acceptable.

Other callers of virtio_gpu_array_lock_resv() (the ioctl paths)
continue to use the interruptible variant.

The bug was reported by syzbot, triggered via fault injection
(fail_nth) on the DRM_IOCTL_MODE_CURSOR path, which forces the
-ENOMEM branch in dma_resv_reserve_fences().

Reported-by: syzbot+72bd3dd3a5d5f39a0271@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=72bd3dd3a5d5f39a0271
Fixes: 5cfd31c5b3a3 ("drm/virtio: fix virtio_gpu_cursor_plane_update().")
Cc: stable@vger.kernel.org
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
v3: Per maintainer feedback on v2 (lockup caused by the broader
    lock scope in prepare_fb conflicting with other BO locking in
    the same atomic commit): drop the prepare_fb/cleanup_fb
    approach, introduce an uninterruptible variant of
    virtio_gpu_array_lock_resv(), and use it in both
    virtio_gpu_cursor_plane_update() and virtio_gpu_resource_flush().
v2: Move resv lock acquisition from .atomic_update (which must not
    fail) to .prepare_fb (which may), per maintainer review of v1.
    The previous approach of silently skipping the cursor update on
    lock failure violated the atomic-commit contract with userspace.
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   |  1 +
 drivers/gpu/drm/virtio/virtgpu_gem.c   | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/virtio/virtgpu_plane.c | 10 ++++++++--
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index f17660a71a3e..43a7eb568e15 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -317,6 +317,7 @@ virtio_gpu_array_from_handles(struct drm_file *drm_file, u32 *handles, u32 nents
 void virtio_gpu_array_add_obj(struct virtio_gpu_object_array *objs,
 			      struct drm_gem_object *obj);
 int virtio_gpu_array_lock_resv(struct virtio_gpu_object_array *objs);
+int virtio_gpu_array_lock_resv_uninterruptible(struct virtio_gpu_object_array *objs);
 void virtio_gpu_array_unlock_resv(struct virtio_gpu_object_array *objs);
 void virtio_gpu_array_add_fence(struct virtio_gpu_object_array *objs,
 				struct dma_fence *fence);
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index f22dc5c21cd4..08c4b7ef8d44 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -238,6 +238,30 @@ int virtio_gpu_array_lock_resv(struct virtio_gpu_object_array *objs)
 	return ret;
 }
 
+int virtio_gpu_array_lock_resv_uninterruptible(struct virtio_gpu_object_array *objs)
+{
+	unsigned int i;
+	int ret = 0;
+
+	if (objs->nents == 1) {
+		dma_resv_lock(objs->objs[0]->resv, NULL);
+	} else {
+		ret = drm_gem_lock_reservations(objs->objs, objs->nents,
+						&objs->ticket);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < objs->nents; ++i) {
+		ret = dma_resv_reserve_fences(objs->objs[i]->resv, 1);
+		if (ret) {
+			virtio_gpu_array_unlock_resv(objs);
+			return ret;
+		}
+	}
+	return 0;
+}
+
 void virtio_gpu_array_unlock_resv(struct virtio_gpu_object_array *objs)
 {
 	if (objs->nents == 1) {
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index a126d1b25f46..ef118cb4f0fa 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -215,7 +215,10 @@ static void virtio_gpu_resource_flush(struct drm_plane *plane,
 		if (!objs)
 			return;
 		virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]);
-		virtio_gpu_array_lock_resv(objs);
+		if (virtio_gpu_array_lock_resv_uninterruptible(objs)) {
+			virtio_gpu_array_put_free(objs);
+			return;
+		}
 		virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, x, y,
 					      width, height, objs,
 					      vgplane_st->fence);
@@ -459,7 +462,10 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane,
 		if (!objs)
 			return;
 		virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]);
-		virtio_gpu_array_lock_resv(objs);
+		if (virtio_gpu_array_lock_resv_uninterruptible(objs)) {
+			virtio_gpu_array_put_free(objs);
+			return;
+		}
 		virtio_gpu_cmd_transfer_to_host_2d
 			(vgdev, 0,
 			 plane->state->crtc_w,
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v2] drm/virtio: move cursor resv lock acquisition to prepare_fb
From: Deepanshu Kartikey @ 2026-05-15  8:41 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: airlied, kraxel, gurchetansingh, olvaffe, maarten.lankhorst,
	mripard, tzimmermann, simona, sumit.semwal, christian.koenig,
	dri-devel, virtualization, linux-kernel, linux-media,
	linaro-mm-sig, syzbot+72bd3dd3a5d5f39a0271, stable
In-Reply-To: <65da2ce9-a2ab-4800-a73e-1a26082d0605@collabora.com>

On Wed, May 13, 2026 at 2:40 PM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> > Does that match what you had in mind?
>
> Sounds good. The virtio_gpu_resource_flush() also should be updated to
> use uninterruptible() variant.
>
> --
> Best regards,
> Dmitry

I  have sent patch v3.

Thanks

Deepanshu

^ permalink raw reply

* Re: [PATCH net v3 1/2] vsock/virtio: reset connection on receiving queue overflow
From: Michael S. Tsirkin @ 2026-05-15  8:57 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: netdev, Xuan Zhuo, Eugenio Pérez, linux-kernel, Simon Horman,
	Paolo Abeni, Jakub Kicinski, Jason Wang, kvm, Stefan Hajnoczi,
	virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <agbWNqhzvEHVBvC2@sgarzare-redhat>

On Fri, May 15, 2026 at 10:29:55AM +0200, Stefano Garzarella wrote:
> On Thu, May 14, 2026 at 01:44:53PM -0400, Michael S. Tsirkin wrote:
> > On Thu, May 14, 2026 at 06:45:00PM +0200, Stefano Garzarella wrote:
> > > On Thu, 14 May 2026 at 17:16, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Thu, May 14, 2026 at 04:57:16PM +0200, Stefano Garzarella wrote:
> > > > > On Wed, May 13, 2026 at 12:54:16PM +0200, Stefano Garzarella wrote:
> > > > > > From: Stefano Garzarella <sgarzare@redhat.com>
> > > > > >
> > > > > > When there is no more space to queue an incoming packet, the packet is
> > > > > > silently dropped. This causes data loss without any notification to
> > > > > > either peer, since there is no retransmission.
> > > > > >
> > > > > > Under normal circumstances, this should never happen. However, it could
> > > > > > happen if the other peer doesn't respect the credit, or if the skb
> > > > > > overhead, which we recently began to take into account with commit
> > > > > > 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue"),
> > > > > > is too high.
> > > > > >
> > > > > > Fix this by resetting the connection and setting the local socket error
> > > > > > to ENOBUFS when virtio_transport_recv_enqueue() can no longer queue a
> > > > > > packet, so both peers are explicitly notified of the failure rather than
> > > > > > silently losing data.
> > > > > >
> > > > > > Fixes: ae6fcfbf5f03 ("vsock/virtio: discard packets if credit is not respected")
> > > > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > > > > ---
> > > > > > net/vmw_vsock/virtio_transport_common.c | 19 ++++++++++++++-----
> > > > > > 1 file changed, 14 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > > > > > index 989cc252d3d3..4a4ac69d1ad1 100644
> > > > > > --- a/net/vmw_vsock/virtio_transport_common.c
> > > > > > +++ b/net/vmw_vsock/virtio_transport_common.c
> > > > > > @@ -1350,7 +1350,7 @@ virtio_transport_recv_connecting(struct sock *sk,
> > > > > >     return err;
> > > > > > }
> > > > > >
> > > > > > -static void
> > > > > > +static bool
> > > > > > virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > > > > >                           struct sk_buff *skb)
> > > > > > {
> > > > > > @@ -1365,10 +1365,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > > > > >     spin_lock_bh(&vvs->rx_lock);
> > > > > >
> > > > > >     can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
> > > > > > -   if (!can_enqueue) {
> > > > > > -           free_pkt = true;
> > > > > > +   if (!can_enqueue)
> > > > > >             goto out;
> > > > > > -   }
> > > > > >
> > > > > >     if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
> > > > > >             vvs->msg_count++;
> > > > > > @@ -1408,6 +1406,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> > > > > >     spin_unlock_bh(&vvs->rx_lock);
> > > > > >     if (free_pkt)
> > > > > >             kfree_skb(skb);
> > > > > > +
> > > > > > +   return can_enqueue;
> > > > > > }
> > > > > >
> > > > > > static int
> > > > > > @@ -1420,7 +1420,16 @@ virtio_transport_recv_connected(struct sock *sk,
> > > > > >
> > > > > >     switch (le16_to_cpu(hdr->op)) {
> > > > > >     case VIRTIO_VSOCK_OP_RW:
> > > > > > -           virtio_transport_recv_enqueue(vsk, skb);
> > > > > > +           if (!virtio_transport_recv_enqueue(vsk, skb)) {
> > > > > > +                   /* There is no more space to queue the packet, so let's
> > > > > > +                    * close the connection; otherwise, we'll lose data.
> > > > > > +                    */
> > > > > > +                   (void)virtio_transport_reset(vsk, skb);
> > > > > > +                   sk->sk_state = TCP_CLOSE;
> > > > > > +                   sk->sk_err = ENOBUFS;
> > > > > > +                   sk_error_report(sk);
> > > > >
> > > > > sashiko reported some issues related to setting TCP_CLOSE state and not
> > > > > removing the socket from the connect table:
> > > > > https://sashiko.dev/#/patchset/20260513105417.56761-1-sgarzare%40redhat.com
> > > > >
> > > > > I'll change this by calling virtio_transport_do_close() and
> > > > > vsock_remove_sock() in the next version.
> > > > >
> > > > > Stefano
> > > > >
> > > > > > +                   break;
> > > > > > +           }
> > > > > >             vsock_data_ready(sk);
> > > > > >             return err;
> > > > > >     case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
> > > > > > --
> > > > > > 2.54.0
> > > > > >
> > > >
> > > >
> > > > And so the bag of hacks grows. I feel this is energy not well spent.
> > > > Please, let us fix this properly *first*. And then worry about how to
> > > > backport.  Maybe it will not be so terrible to backport after all.
> > > >
> > > 
> > > TBH I don't think this is an hack, but an issue we should fix in any case.
> > > Regarding the second patch, I see your point, but it's a big change
> > > that worries me. I'd like some more time to fix it properly without
> > > rushing. Staying calm without realizing that userspace is broken like
> > > we are now without this series :-(
> > > 
> > > That said, evaluating further, I think we have a similar issue also
> > > with STREAM on the host side where the skb usually doesn't free space,
> > > so we need a merge strategy also there.
> > > 
> > > So, I'd like to have time to fix both definitely. If you have time and
> > > want to go ahead, please do.
> > > 
> > > Thanks,
> > > Stefano
> > 
> > Well my patch was a start, we just need a strategy how to avoid copying
> > everything, right?
> 
> Yep, and then there's the question of how to handle EOM without a payload,
> but I think that's a special case. In theory, we don't support sending it,
> but I'm not sure if POSIX allows it or not.

It seems to, but given we didn't allow it in the past, we probably
should not start now without a good solution.
Really we should add a feature bit for EOM to steal a byte from
buf_alloc. Or several bytes)

> That said, is it okay if I send a v4 of this series?
> 
> (I'm not sure if I'll be able to work on the merging next week)
> 
> Stefano


I do worry we are piling up hacks and we'll end up with races
for all our troubles. That said, up to you.


^ permalink raw reply

* Re: [PATCH net v3 1/2] vsock/virtio: reset connection on receiving queue overflow
From: Stefano Garzarella @ 2026-05-15  9:06 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, Xuan Zhuo, Eugenio Pérez, linux-kernel, Simon Horman,
	Paolo Abeni, Jakub Kicinski, Jason Wang, kvm, Stefan Hajnoczi,
	virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <20260515043940-mutt-send-email-mst@kernel.org>

On Fri, May 15, 2026 at 04:57:41AM -0400, Michael S. Tsirkin wrote:
>On Fri, May 15, 2026 at 10:29:55AM +0200, Stefano Garzarella wrote:
>> On Thu, May 14, 2026 at 01:44:53PM -0400, Michael S. Tsirkin wrote:
>> > On Thu, May 14, 2026 at 06:45:00PM +0200, Stefano Garzarella wrote:
>> > > On Thu, 14 May 2026 at 17:16, Michael S. Tsirkin <mst@redhat.com> wrote:

[...]

>> > > >
>> > > >
>> > > > And so the bag of hacks grows. I feel this is energy not well spent.
>> > > > Please, let us fix this properly *first*. And then worry about how to
>> > > > backport.  Maybe it will not be so terrible to backport after all.
>> > > >
>> > >
>> > > TBH I don't think this is an hack, but an issue we should fix in any case.
>> > > Regarding the second patch, I see your point, but it's a big change
>> > > that worries me. I'd like some more time to fix it properly without
>> > > rushing. Staying calm without realizing that userspace is broken like
>> > > we are now without this series :-(
>> > >
>> > > That said, evaluating further, I think we have a similar issue also
>> > > with STREAM on the host side where the skb usually doesn't free space,
>> > > so we need a merge strategy also there.
>> > >
>> > > So, I'd like to have time to fix both definitely. If you have time and
>> > > want to go ahead, please do.
>> > >
>> > > Thanks,
>> > > Stefano
>> >
>> > Well my patch was a start, we just need a strategy how to avoid copying
>> > everything, right?
>>
>> Yep, and then there's the question of how to handle EOM without a payload,
>> but I think that's a special case. In theory, we don't support sending it,
>> but I'm not sure if POSIX allows it or not.
>
>It seems to, but given we didn't allow it in the past, we probably
>should not start now without a good solution.

Agree.

>Really we should add a feature bit for EOM to steal a byte from
>buf_alloc. Or several bytes)

Yes, I agree. At this point, though, could we define a new protocol that 
also takes overhead into account, or would it be too complicated to 
synchronize both?

>
>> That said, is it okay if I send a v4 of this series?
>>
>> (I'm not sure if I'll be able to work on the merging next week)
>>
>> Stefano
>
>
>I do worry we are piling up hacks and we'll end up with races
>for all our troubles. That said, up to you.
>

I see it differently; Patch 1 should have been there from the start.
Patch 2, unless we completely remove the overhead, we should keep it, or 
use it to trigger merging (e.g., when the overhead reaches a threshold 
that depends on `buf_alloc`).

I prefer to send a v4.

Thanks,
Stefano


^ permalink raw reply

* Re: [PATCH net v3 1/2] vsock/virtio: reset connection on receiving queue overflow
From: Michael S. Tsirkin @ 2026-05-15  9:20 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: netdev, Xuan Zhuo, Eugenio Pérez, linux-kernel, Simon Horman,
	Paolo Abeni, Jakub Kicinski, Jason Wang, kvm, Stefan Hajnoczi,
	virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <agbg-pUhBRB89aEX@sgarzare-redhat>

On Fri, May 15, 2026 at 11:06:42AM +0200, Stefano Garzarella wrote:
> On Fri, May 15, 2026 at 04:57:41AM -0400, Michael S. Tsirkin wrote:
> > On Fri, May 15, 2026 at 10:29:55AM +0200, Stefano Garzarella wrote:
> > > On Thu, May 14, 2026 at 01:44:53PM -0400, Michael S. Tsirkin wrote:
> > > > On Thu, May 14, 2026 at 06:45:00PM +0200, Stefano Garzarella wrote:
> > > > > On Thu, 14 May 2026 at 17:16, Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> [...]
> 
> > > > > >
> > > > > >
> > > > > > And so the bag of hacks grows. I feel this is energy not well spent.
> > > > > > Please, let us fix this properly *first*. And then worry about how to
> > > > > > backport.  Maybe it will not be so terrible to backport after all.
> > > > > >
> > > > >
> > > > > TBH I don't think this is an hack, but an issue we should fix in any case.
> > > > > Regarding the second patch, I see your point, but it's a big change
> > > > > that worries me. I'd like some more time to fix it properly without
> > > > > rushing. Staying calm without realizing that userspace is broken like
> > > > > we are now without this series :-(
> > > > >
> > > > > That said, evaluating further, I think we have a similar issue also
> > > > > with STREAM on the host side where the skb usually doesn't free space,
> > > > > so we need a merge strategy also there.
> > > > >
> > > > > So, I'd like to have time to fix both definitely. If you have time and
> > > > > want to go ahead, please do.
> > > > >
> > > > > Thanks,
> > > > > Stefano
> > > >
> > > > Well my patch was a start, we just need a strategy how to avoid copying
> > > > everything, right?
> > > 
> > > Yep, and then there's the question of how to handle EOM without a payload,
> > > but I think that's a special case. In theory, we don't support sending it,
> > > but I'm not sure if POSIX allows it or not.
> > 
> > It seems to, but given we didn't allow it in the past, we probably
> > should not start now without a good solution.
> 
> Agree.
> 
> > Really we should add a feature bit for EOM to steal a byte from
> > buf_alloc. Or several bytes)
> 
> Yes, I agree. At this point, though, could we define a new protocol that
> also takes overhead into account, or would it be too complicated to
> synchronize both?

Well isn't it just X bytes per EOM? The rest can be fixed with
merging?

> > 
> > > That said, is it okay if I send a v4 of this series?
> > > 
> > > (I'm not sure if I'll be able to work on the merging next week)
> > > 
> > > Stefano
> > 
> > 
> > I do worry we are piling up hacks and we'll end up with races
> > for all our troubles. That said, up to you.
> > 
> 
> I see it differently; Patch 1 should have been there from the start.
> Patch 2, unless we completely remove the overhead, we should keep it, or use
> it to trigger merging (e.g., when the overhead reaches a threshold that
> depends on `buf_alloc`).

I'm not sure we need to merge retroactively though. Maybe just start
merging new stuff when over a threshold. Should be simpler.


> I prefer to send a v4.
> 
> Thanks,
> Stefano


^ permalink raw reply

* Re: [PATCH] ALSA: virtio: Validate control metadata from the device
From: Takashi Iwai @ 2026-05-15  9:21 UTC (permalink / raw)
  To: Cássio Gabriel
  Cc: Takashi Iwai, Anton Yakovlev, Michael S. Tsirkin, Aiswarya Cyriac,
	Jaroslav Kysela, virtualization, linux-sound, linux-kernel,
	stable
In-Reply-To: <20260507-alsa-virtio-validate-kctl-info-v1-1-7404fb12ec37@gmail.com>

On Thu, 07 May 2026 16:28:30 +0200,
Cássio Gabriel wrote:
> 
> virtio-snd control handling trusts the device-provided control type and
> value count returned by the device.
> 
> That metadata is then used directly to index g_v2a_type_map[] in
> virtsnd_kctl_info(), and to size loops and memcpy() operations in
> virtsnd_kctl_get() and virtsnd_kctl_put() against fixed-size
> virtio_snd_ctl_value and snd_ctl_elem_value arrays.
> 
> A buggy or malicious device can therefore trigger out-of-bounds access by
> advertising an invalid control type or an oversized value count.
> 
> Validate control type and count once in virtsnd_kctl_parse_cfg(), before
> querying enumerated items or exposing the control to ALSA.
> 
> Fixes: d6568e3de42d ("ALSA: virtio: add support for audio controls")
> Cc: stable@vger.kernel.org
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>

Applied to for-next branch now.  Thanks.


Takashi

^ permalink raw reply

* Re: [PATCH v2] vhost/vsock: Refuse the connection immediately when guest isn't ready
From: Stefano Garzarella @ 2026-05-15  9:33 UTC (permalink / raw)
  To: Polina Vishneva, mst
  Cc: linux-kernel, netdev, virtualization, kvm, Eugenio Pérez,
	Jason Wang, Michael S. Tsirkin, Stefan Hajnoczi, Denis V . Lunev
In-Reply-To: <20260513145842.809404-1-polina.vishneva@virtuozzo.com>

On Wed, May 13, 2026 at 04:50:04PM +0200, Polina Vishneva wrote:
>From: "Denis V. Lunev" <den@openvz.org>
>
>When the host initiates an AF_VSOCK connect() to a guest that has not
>yet loaded the virtio-vsock transport (i.e. still booting), the caller
>blocks for VSOCK_DEFAULT_CONNECT_TIMEOUT.
>
>A caller that wants to know if the guest is up yet instead of waiting
>could theoretically tune SO_VM_SOCKETS_CONNECT_TIMEOUT, but it's tricky
>to find the right timeout, if not impossible: there's no way to
>distinguish "guest won't reply because it's not up yet" vs "guest is up
>and tried to reply, but was too slow".
>
>Furthermore, this delay is pointless:
>- If the guest doesn't initialize within this timeout, connect()
>  returns ETIMEDOUT.
>- If the guest **does** initialize, it'll reply with RST immediately,
>  because there won't be a listener on the port yet; connect() returns
>  ECONNRESET.
>
>That's also inconsistent with the behavior at other initialization
>stages: if a connection is attempted when the guest driver is already
>loaded, but nothing is listening yet, we return ECONNRESET immediately
>without waiting.
>
>Fix this by checking the RX virtqueue backend in
>vhost_transport_send_pkt() before queuing. If it's NULL, return
>-EHOSTUNREACH immediately.
>
>Callers that used to get ETIMEDOUT will now usually get EHOSTUNREACH.
>
>Signed-off-by: Denis V. Lunev <den@openvz.org>
>Co-developed-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
>Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
>---
>v2:
>- ECONNREFUSED -> EHOSTUNREACH.
>- Use vhost_transport_do_send_pkt() instead of raw .private_data access.
>- Removed READ_ONCE().
>- Wrapped the condition with unlikely().
>- Updated the comment and the commit message.
>
>v1: https://lore.kernel.org/netdev/20260511145610.413210-1-polina.vishneva@virtuozzo.com
>
> drivers/vhost/vsock.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

@Michael I think this can go with your tree, right?

(I don't see it in https://patchwork.kernel.org/project/netdevbpf/list/)

Thanks,
Stefano


^ permalink raw reply

* [PATCH 0/9] drm: Limit DRM_IOCTL_WAIT_VBLANK to vblank interrupts
From: Thomas Zimmermann @ 2026-05-15 11:55 UTC (permalink / raw)
  To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
	maarten.lankhorst, mripard
  Cc: amd-gfx, dri-devel, linux-hyperv, virtualization, spice-devel,
	Thomas Zimmermann

DRM's WAIT_VBLANK ioctl synchronizes user-space clients to display
refresh. This is meaningless with vblank timers, which run unrelated
to the hardware's vblank.

Disable the ioctl for simulated vblanks. Set DRM_VBLANK_FLAG_SIMULATED
for CRTCs with simulated vblank events in all such drivers. The vblank
timers of these devices still rate-limit the number of page-flip events
to match the display refresh.

According to maintainers, user-space compositors do not require the ioctl
for rate-limitting display output. Weston and Kwin rely on page-flip
events. Mutter uses and internal timer to limit the number of display
updates per second.

When testing with mutter and weston, the page-flip rate appears correct
with the patch set applied.

This change has been discussed at length on IRC recently.

https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2026-05-08&show_html=true
https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2026-05-12&show_html=true
https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2026-05-13&show_html=true

Thomas Zimmermann (9):
  drm/vblank: Add drmm_vblank_init() to indicate managed cleanup
  drm/vblank: Add DRM_VBLANK_FLAG_SIMULATED
  drm/amdgpu: vkms: Set DRM_VBLANK_FLAG_SIMULATED
  drm/bochs: Set DRM_VBLANK_FLAG_SIMULATED
  drm/cirrus: Set DRM_VBLANK_FLAG_SIMULATED
  drm/hypervdrm: Set DRM_VBLANK_FLAG_SIMULATED
  drm/qxl: Set DRM_VBLANK_FLAG_SIMULATED
  drm/virtgpu: Set DRM_VBLANK_FLAG_SIMULATED
  drm/vkms: Set DRM_VBLANK_FLAG_SIMULATED

 drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c    |  3 ++-
 drivers/gpu/drm/drm_vblank.c                | 19 ++++++++++++-------
 drivers/gpu/drm/drm_vblank_helper.c         |  2 +-
 drivers/gpu/drm/hyperv/hyperv_drm_modeset.c |  2 +-
 drivers/gpu/drm/qxl/qxl_display.c           |  2 +-
 drivers/gpu/drm/tiny/bochs.c                |  2 +-
 drivers/gpu/drm/tiny/cirrus-qemu.c          |  2 +-
 drivers/gpu/drm/virtio/virtgpu_display.c    |  2 +-
 drivers/gpu/drm/vkms/vkms_drv.c             |  4 ++--
 include/drm/drm_crtc.h                      |  2 +-
 include/drm/drm_device.h                    |  2 +-
 include/drm/drm_vblank.h                    | 15 ++++++++++++++-
 12 files changed, 38 insertions(+), 19 deletions(-)


base-commit: 121c16f9d8c56ea07263df84ab971cc10870fe88
-- 
2.54.0


^ permalink raw reply

* [PATCH 1/9] drm/vblank: Add drmm_vblank_init() to indicate managed cleanup
From: Thomas Zimmermann @ 2026-05-15 11:55 UTC (permalink / raw)
  To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
	maarten.lankhorst, mripard
  Cc: amd-gfx, dri-devel, linux-hyperv, virtualization, spice-devel,
	Thomas Zimmermann
In-Reply-To: <20260515120916.333614-1-tzimmermann@suse.de>

Rename drm_vblank_init() to drmm_vblank_init(). As the initializer
function sets up managed cleanup, it should use the drmm prefix. Keep
the old name around until all callers have been converted.

Also add a flags argument to the function. The first use of the flags
will be to distinguish between hardware vblank interrupts and simulated
vblank timeouts.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_vblank.c        | 16 +++++++++-------
 drivers/gpu/drm/drm_vblank_helper.c |  2 +-
 include/drm/drm_crtc.h              |  2 +-
 include/drm/drm_device.h            |  2 +-
 include/drm/drm_vblank.h            | 10 +++++++++-
 5 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index f90fb2d13e42..21ca91b4c014 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -117,7 +117,7 @@
  * optionally provide a hardware vertical blanking counter.
  *
  * Drivers must initialize the vertical blanking handling core with a call to
- * drm_vblank_init(). Minimally, a driver needs to implement
+ * drmm_vblank_init(). Minimally, a driver needs to implement
  * &drm_crtc_funcs.enable_vblank and &drm_crtc_funcs.disable_vblank plus call
  * drm_crtc_handle_vblank() in its vblank interrupt handler for working vblank
  * support.
@@ -146,7 +146,7 @@
  * See also DRM vblank helpers for more information.
  *
  * Drivers without support for vertical-blanking interrupts nor timers must
- * not call drm_vblank_init(). For these drivers, atomic helpers will
+ * not call drmm_vblank_init(). For these drivers, atomic helpers will
  * automatically generate fake vblank events as part of the display update.
  * This functionality also can be controlled by the driver by enabling and
  * disabling struct drm_crtc_state.no_vblank.
@@ -519,7 +519,7 @@ static void vblank_disable_fn(struct timer_list *t)
 	spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
 }
 
-static void drm_vblank_init_release(struct drm_device *dev, void *ptr)
+static void drmm_vblank_init_release(struct drm_device *dev, void *ptr)
 {
 	struct drm_vblank_crtc *vblank = ptr;
 
@@ -534,9 +534,10 @@ static void drm_vblank_init_release(struct drm_device *dev, void *ptr)
 }
 
 /**
- * drm_vblank_init - initialize vblank support
+ * drmm_vblank_init - initialize vblank support
  * @dev: DRM device
  * @num_crtcs: number of CRTCs supported by @dev
+ * @flags: flags for vblank handling
  *
  * This function initializes vblank support for @num_crtcs display pipelines.
  * Cleanup is handled automatically through a cleanup function added with
@@ -545,7 +546,7 @@ static void drm_vblank_init_release(struct drm_device *dev, void *ptr)
  * Returns:
  * Zero on success or a negative error code on failure.
  */
-int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
+int drmm_vblank_init(struct drm_device *dev, unsigned int num_crtcs, unsigned int flags)
 {
 	int ret;
 	unsigned int i;
@@ -564,11 +565,12 @@ int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
 
 		vblank->dev = dev;
 		vblank->pipe = i;
+		vblank->flags = flags;
 		init_waitqueue_head(&vblank->queue);
 		timer_setup(&vblank->disable_timer, vblank_disable_fn, 0);
 		seqlock_init(&vblank->seqlock);
 
-		ret = drmm_add_action_or_reset(dev, drm_vblank_init_release,
+		ret = drmm_add_action_or_reset(dev, drmm_vblank_init_release,
 					       vblank);
 		if (ret)
 			return ret;
@@ -580,7 +582,7 @@ int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
 
 	return 0;
 }
-EXPORT_SYMBOL(drm_vblank_init);
+EXPORT_SYMBOL(drmm_vblank_init);
 
 /**
  * drm_dev_has_vblank - test if vblanking has been initialized for
diff --git a/drivers/gpu/drm/drm_vblank_helper.c b/drivers/gpu/drm/drm_vblank_helper.c
index d3f8147ecdc1..5b05ab72e133 100644
--- a/drivers/gpu/drm/drm_vblank_helper.c
+++ b/drivers/gpu/drm/drm_vblank_helper.c
@@ -25,7 +25,7 @@
  * for drivers without further requirements. The initializer macro
  * DRM_CRTC_HELPER_VBLANK_FUNCS sets them coveniently.
  *
- * Once the driver enables vblank support with drm_vblank_init(), each
+ * Once the driver enables vblank support with drmm_vblank_init(), each
  * CRTC's vblank timer fires according to the programmed display mode. By
  * default, the vblank timer invokes drm_crtc_handle_vblank(). Drivers with
  * more specific requirements can set their own handler function in
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c6dbe8b7db9e..f981468d9a00 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -163,7 +163,7 @@ struct drm_crtc_state {
 	 *
 	 * One usage is for drivers and/or hardware without support for VBLANK
 	 * interrupts. Such drivers typically do not initialize vblanking
-	 * (i.e., call drm_vblank_init() with the number of CRTCs). For CRTCs
+	 * (i.e., call drmm_vblank_init() with the number of CRTCs). For CRTCs
 	 * without initialized vblanking, this field is set to true in
 	 * drm_atomic_helper_check_modeset(), and a fake VBLANK event will be
 	 * send out on each update of the display pipeline by
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index bc78fb77cc27..381417c6b6f5 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -283,7 +283,7 @@ struct drm_device {
 	 * Array of vblank tracking structures, one per &struct drm_crtc. For
 	 * historical reasons (vblank support predates kernel modesetting) this
 	 * is free-standing and not part of &struct drm_crtc itself. It must be
-	 * initialized explicitly by calling drm_vblank_init().
+	 * initialized explicitly by calling drmm_vblank_init().
 	 */
 	struct drm_vblank_crtc *vblank;
 
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 2fcef9c0f5b1..39a201b83781 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -282,10 +282,12 @@ struct drm_vblank_crtc {
 	 * @vblank_timer: Holds the state of the vblank timer
 	 */
 	struct drm_vblank_crtc_timer vblank_timer;
+
+	unsigned int flags;
 };
 
 struct drm_vblank_crtc *drm_crtc_vblank_crtc(struct drm_crtc *crtc);
-int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs);
+int drmm_vblank_init(struct drm_device *dev, unsigned int num_crtcs, unsigned int flags);
 bool drm_dev_has_vblank(const struct drm_device *dev);
 u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
 u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
@@ -321,6 +323,12 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc);
 void drm_crtc_vblank_cancel_timer(struct drm_crtc *crtc);
 void drm_crtc_vblank_get_vblank_timeout(struct drm_crtc *crtc, ktime_t *vblank_time);
 
+/* deprecated; use drmm_vblank_init() instead */
+static inline int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
+{
+	return drmm_vblank_init(dev, num_crtcs, 0);
+}
+
 /*
  * Helpers for struct drm_crtc_funcs
  */
-- 
2.54.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox