* Re: [PATCH net-next v9 4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
From: Michael S. Tsirkin @ 2026-04-28 13:22 UTC (permalink / raw)
To: Simon Schippers
Cc: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, eperezma, leiyang, stephen, jon, tim.gebauer,
netdev, linux-kernel, kvm, virtualization
In-Reply-To: <cdc3cbc8-cf6a-437c-b34c-dd8ced65d7f8@tu-dortmund.de>
On Tue, Apr 28, 2026 at 03:10:44PM +0200, Simon Schippers wrote:
> On 4/28/26 14:50, Michael S. Tsirkin wrote:
> > On Tue, Apr 28, 2026 at 02:38:59PM +0200, Simon Schippers wrote:
> >> This commit prevents tail-drop when a qdisc is present and the ptr_ring
> >> becomes full. Once an entry is successfully produced and the ptr_ring
> >> reaches capacity, the netdev queue is stopped instead of dropping
> >> subsequent packets.
> >>
> >> If producing an entry fails anyways due to a race, tun_net_xmit returns
> >> NETDEV_TX_BUSY, again avoiding a drop. Such races are expected because
> >> LLTX is enabled and the transmit path operates without the usual locking.
> >>
> >> If no qdisc is present, the previous tail-drop behavior is preserved.
> >>
> >> The existing __tun_wake_queue() function of the consumer races with the
> >> producer for waking/stopping the netdev queue: the consumer may drain
> >> the ring just as the producer stops the queue, leading to a permanent
> >> stall. To avoid this, the producer re-checks the ring after stopping
> >> and wakes the queue itself if space was just made. An
> >> smp_mb__after_atomic() is required so the re-peek of the ring sees any
> >> drain that the consumer performed.
> >> smp_mb__after_atomic() pairs with the test_and_clear_bit() inside of
> >> netif_wake_subqueue():
> >>
> >> Consumer CPU Producer CPU
> >> ======================== =========================
> >> __ptr_ring_consume()
> >> netif_wake_subqueue() netif_tx_stop_queue()
> >> /\ smp_mb__after_atomic()
> >> || __ptr_ring_produce_peek()
> >> contains RMW operation
> >> test_and_clear_bit()
> >> /\
> >> ||
> >> "Fully ordered RMW:
> >> smp_mb() before + after"
> >> - atomic_t.txt
> >>
> >> Benchmarks:
> >> The benchmarks show a slight regression in raw transmission performance,
> >> though no packets are lost anymore.
> >
> > Could you include the packets received as well?
> > To demonstrate the gains/lack of loss.
> >
>
> Do you mean the number of packets received by the VM?
> They should just be the same as the number sent (shown below), right?
Minus the loss? Which this is about, right?
> I assume they would be visible as RX-DRP for TAP.
> For TAP + vhost-net I would have to rewrite the XDP drop
> program to count the number of dropped packets...
> And I would have to automate it...
>
> >>
> >> The previously introduced threshold to only wake after the queue stopped
> >> and half of the ring was consumed showed to be a descent choice:
> >> Waking the queue whenever a consume made space in the ring strongly
> >> degrades performance for tap, while waking only when the ring is empty
> >> is too late and also hurts throughput for tap & tap+vhost-net.
> >> Other ratios (3/4, 7/8) showed similar results (not shown here), so
> >> 1/2 was chosen for the sake of simplicity for both tun/tap and
> >> tun/tap+vhost-net.
> >>
> >> Test setup:
> >> AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU threads;
> >> Average over 50 runs @ 100,000,000 packets. SRSO and spectre v2
> >> mitigations disabled.
> >>
> >> Note for tap+vhost-net:
> >> XDP drop program active in VM -> ~2.5x faster, slower for tap due to
> >> more syscalls (high utilization of entry_SYSRETQ_unsafe_stack in perf)
> >>
> >> +--------------------------+--------------+----------------+----------+
> >> | 1 thread | Stock | Patched with | diff |
> >> | sending | | fq_codel qdisc | |
> >> +------------+-------------+--------------+----------------+----------+
> >> | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
> >> | +-------------+--------------+----------------+----------+
> >> | | Lost/s | 3.758 Mpps | 0 pps | |
> >> +------------+-------------+--------------+----------------+----------+
> >> | TAP | Transmitted | 3.858 Mpps | 3.816 Mpps | -1.1% |
> >> | +-------------+--------------+----------------+----------+
> >> | +vhost-net | Lost/s | 789.8 Kpps | 0 pps | |
> >> +------------+-------------+--------------+----------------+----------+
> >>
> >> +--------------------------+--------------+----------------+----------+
> >> | 2 threads | Stock | Patched with | diff |
> >> | sending | | fq_codel qdisc | |
> >> +------------+-------------+--------------+----------------+----------+
> >> | TAP | Transmitted | 1.117 Mpps | 1.087 Mpps | -2.7% |
> >> | +-------------+--------------+----------------+----------+
> >> | | Lost/s | 8.476 Mpps | 0 pps | |
> >> +------------+-------------+--------------+----------------+----------+
> >> | TAP | Transmitted | 3.679 Mpps | 3.464 Mpps | -5.8% |
> >> | +-------------+--------------+----------------+----------+
> >> | +vhost-net | Lost/s | 5.306 Mpps | 0 pps | |
> >> +------------+-------------+--------------+----------------+----------+
> >>
> >> Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
> >> Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
> >> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
> >> ---
> >> drivers/net/tun.c | 30 ++++++++++++++++++++++++++++--
> >> 1 file changed, 28 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >> index efe809597622..c2a1618cc9db 100644
> >> --- a/drivers/net/tun.c
> >> +++ b/drivers/net/tun.c
> >> @@ -1011,6 +1011,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >> struct netdev_queue *queue;
> >> struct tun_file *tfile;
> >> int len = skb->len;
> >> + bool qdisc_present;
> >> + int ret;
> >>
> >> rcu_read_lock();
> >> tfile = rcu_dereference(tun->tfiles[txq]);
> >> @@ -1065,13 +1067,37 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >>
> >> nf_reset_ct(skb);
> >>
> >> - if (ptr_ring_produce(&tfile->tx_ring, skb)) {
> >> + queue = netdev_get_tx_queue(dev, txq);
> >> + qdisc_present = !qdisc_txq_has_no_queue(queue);
> >> +
> >> + spin_lock(&tfile->tx_ring.producer_lock);
> >> + ret = __ptr_ring_produce(&tfile->tx_ring, skb);
> >> + if (__ptr_ring_produce_peek(&tfile->tx_ring) && qdisc_present) {
> >> + netif_tx_stop_queue(queue);
> >> + /* Re-peek and wake if the consumer drained the ring
> >> + * concurrently in a race. smp_mb__after_atomic() pairs
> >> + * with the test_and_clear_bit() of netif_wake_subqueue()
> >> + * in __tun_wake_queue().
> >> + */
> >> + smp_mb__after_atomic();
> >> + if (!__ptr_ring_produce_peek(&tfile->tx_ring))
> >> + netif_tx_wake_queue(queue);
> >> + }
> >> + spin_unlock(&tfile->tx_ring.producer_lock);
> >> +
> >> + if (ret) {
> >> + /* If a qdisc is attached to our virtual device,
> >> + * returning NETDEV_TX_BUSY is allowed.
> >> + */
> >> + if (qdisc_present) {
> >> + rcu_read_unlock();
> >> + return NETDEV_TX_BUSY;
> >> + }
> >> drop_reason = SKB_DROP_REASON_FULL_RING;
> >> goto drop;
> >> }
> >>
> >> /* dev->lltx requires to do our own update of trans_start */
> >> - queue = netdev_get_tx_queue(dev, txq);
> >> txq_trans_cond_update(queue);
> >>
> >> /* Notify and wake up reader process */
> >> --
> >> 2.43.0
> >
^ permalink raw reply
* Re: [PATCH] hv_sock: fix ARM64 support
From: Stefano Garzarella @ 2026-04-28 13:12 UTC (permalink / raw)
To: Hamza Mahfooz
Cc: netdev, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Long Li, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Michael Kelley, Himadri Pandya,
linux-hyperv, virtualization, linux-kernel
In-Reply-To: <20260428125339.13963-1-hamzamahfooz@linux.microsoft.com>
No version number in the subject?
Please next time follow
https://docs.kernel.org/process/submitting-patches.html#subject-line
Common tags might include a version descriptor if the multiple
versions of the patch have been sent out in response to comments
(i.e., “v1, v2, v3”), or “RFC” to indicate a request for comments.
On Tue, Apr 28, 2026 at 08:53:39AM -0400, Hamza Mahfooz wrote:
>VMBUS ring buffers must be page aligned. Therefore, the current value of
>24K presents a challenge on ARM64 kernels (with 64K pages). So, use
>VMBUS_RING_SIZE() to ensure they are always aligned and large enough to
>hold all of the relevant data.
>
>Cc: stable@vger.kernel.org
>Fixes: 77ffe33363c0 ("hv_sock: use HV_HYP_PAGE_SIZE for Hyper-V communication")
>Tested-by: Dexuan Cui <decui@microsoft.com>
>Reviewed-by: Dexuan Cui <decui@microsoft.com>
>Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
>---
> net/vmw_vsock/hyperv_transport.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Acked-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 069386a74557..40f09b23efa3 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -375,10 +375,10 @@ static void hvs_open_connection(struct vmbus_channel *chan)
> } else {
> sndbuf = max_t(int, sk->sk_sndbuf, RINGBUFFER_HVS_SND_SIZE);
> sndbuf = min_t(int, sndbuf, RINGBUFFER_HVS_MAX_SIZE);
>- sndbuf = ALIGN(sndbuf, HV_HYP_PAGE_SIZE);
>+ sndbuf = VMBUS_RING_SIZE(sndbuf);
> rcvbuf = max_t(int, sk->sk_rcvbuf, RINGBUFFER_HVS_RCV_SIZE);
> rcvbuf = min_t(int, rcvbuf, RINGBUFFER_HVS_MAX_SIZE);
>- rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
>+ rcvbuf = VMBUS_RING_SIZE(rcvbuf);
> }
>
> chan->max_pkt_size = HVS_MAX_PKT_SIZE;
>--
>2.54.0
>
^ permalink raw reply
* Re: [PATCH net-next v9 4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
From: Simon Schippers @ 2026-04-28 13:10 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, eperezma, leiyang, stephen, jon, tim.gebauer,
netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260428084851-mutt-send-email-mst@kernel.org>
On 4/28/26 14:50, Michael S. Tsirkin wrote:
> On Tue, Apr 28, 2026 at 02:38:59PM +0200, Simon Schippers wrote:
>> This commit prevents tail-drop when a qdisc is present and the ptr_ring
>> becomes full. Once an entry is successfully produced and the ptr_ring
>> reaches capacity, the netdev queue is stopped instead of dropping
>> subsequent packets.
>>
>> If producing an entry fails anyways due to a race, tun_net_xmit returns
>> NETDEV_TX_BUSY, again avoiding a drop. Such races are expected because
>> LLTX is enabled and the transmit path operates without the usual locking.
>>
>> If no qdisc is present, the previous tail-drop behavior is preserved.
>>
>> The existing __tun_wake_queue() function of the consumer races with the
>> producer for waking/stopping the netdev queue: the consumer may drain
>> the ring just as the producer stops the queue, leading to a permanent
>> stall. To avoid this, the producer re-checks the ring after stopping
>> and wakes the queue itself if space was just made. An
>> smp_mb__after_atomic() is required so the re-peek of the ring sees any
>> drain that the consumer performed.
>> smp_mb__after_atomic() pairs with the test_and_clear_bit() inside of
>> netif_wake_subqueue():
>>
>> Consumer CPU Producer CPU
>> ======================== =========================
>> __ptr_ring_consume()
>> netif_wake_subqueue() netif_tx_stop_queue()
>> /\ smp_mb__after_atomic()
>> || __ptr_ring_produce_peek()
>> contains RMW operation
>> test_and_clear_bit()
>> /\
>> ||
>> "Fully ordered RMW:
>> smp_mb() before + after"
>> - atomic_t.txt
>>
>> Benchmarks:
>> The benchmarks show a slight regression in raw transmission performance,
>> though no packets are lost anymore.
>
> Could you include the packets received as well?
> To demonstrate the gains/lack of loss.
>
Do you mean the number of packets received by the VM?
They should just be the same as the number sent (shown below), right?
I assume they would be visible as RX-DRP for TAP.
For TAP + vhost-net I would have to rewrite the XDP drop
program to count the number of dropped packets...
And I would have to automate it...
>>
>> The previously introduced threshold to only wake after the queue stopped
>> and half of the ring was consumed showed to be a descent choice:
>> Waking the queue whenever a consume made space in the ring strongly
>> degrades performance for tap, while waking only when the ring is empty
>> is too late and also hurts throughput for tap & tap+vhost-net.
>> Other ratios (3/4, 7/8) showed similar results (not shown here), so
>> 1/2 was chosen for the sake of simplicity for both tun/tap and
>> tun/tap+vhost-net.
>>
>> Test setup:
>> AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU threads;
>> Average over 50 runs @ 100,000,000 packets. SRSO and spectre v2
>> mitigations disabled.
>>
>> Note for tap+vhost-net:
>> XDP drop program active in VM -> ~2.5x faster, slower for tap due to
>> more syscalls (high utilization of entry_SYSRETQ_unsafe_stack in perf)
>>
>> +--------------------------+--------------+----------------+----------+
>> | 1 thread | Stock | Patched with | diff |
>> | sending | | fq_codel qdisc | |
>> +------------+-------------+--------------+----------------+----------+
>> | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
>> | +-------------+--------------+----------------+----------+
>> | | Lost/s | 3.758 Mpps | 0 pps | |
>> +------------+-------------+--------------+----------------+----------+
>> | TAP | Transmitted | 3.858 Mpps | 3.816 Mpps | -1.1% |
>> | +-------------+--------------+----------------+----------+
>> | +vhost-net | Lost/s | 789.8 Kpps | 0 pps | |
>> +------------+-------------+--------------+----------------+----------+
>>
>> +--------------------------+--------------+----------------+----------+
>> | 2 threads | Stock | Patched with | diff |
>> | sending | | fq_codel qdisc | |
>> +------------+-------------+--------------+----------------+----------+
>> | TAP | Transmitted | 1.117 Mpps | 1.087 Mpps | -2.7% |
>> | +-------------+--------------+----------------+----------+
>> | | Lost/s | 8.476 Mpps | 0 pps | |
>> +------------+-------------+--------------+----------------+----------+
>> | TAP | Transmitted | 3.679 Mpps | 3.464 Mpps | -5.8% |
>> | +-------------+--------------+----------------+----------+
>> | +vhost-net | Lost/s | 5.306 Mpps | 0 pps | |
>> +------------+-------------+--------------+----------------+----------+
>>
>> Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
>> Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
>> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
>> ---
>> drivers/net/tun.c | 30 ++++++++++++++++++++++++++++--
>> 1 file changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index efe809597622..c2a1618cc9db 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -1011,6 +1011,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>> struct netdev_queue *queue;
>> struct tun_file *tfile;
>> int len = skb->len;
>> + bool qdisc_present;
>> + int ret;
>>
>> rcu_read_lock();
>> tfile = rcu_dereference(tun->tfiles[txq]);
>> @@ -1065,13 +1067,37 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>>
>> nf_reset_ct(skb);
>>
>> - if (ptr_ring_produce(&tfile->tx_ring, skb)) {
>> + queue = netdev_get_tx_queue(dev, txq);
>> + qdisc_present = !qdisc_txq_has_no_queue(queue);
>> +
>> + spin_lock(&tfile->tx_ring.producer_lock);
>> + ret = __ptr_ring_produce(&tfile->tx_ring, skb);
>> + if (__ptr_ring_produce_peek(&tfile->tx_ring) && qdisc_present) {
>> + netif_tx_stop_queue(queue);
>> + /* Re-peek and wake if the consumer drained the ring
>> + * concurrently in a race. smp_mb__after_atomic() pairs
>> + * with the test_and_clear_bit() of netif_wake_subqueue()
>> + * in __tun_wake_queue().
>> + */
>> + smp_mb__after_atomic();
>> + if (!__ptr_ring_produce_peek(&tfile->tx_ring))
>> + netif_tx_wake_queue(queue);
>> + }
>> + spin_unlock(&tfile->tx_ring.producer_lock);
>> +
>> + if (ret) {
>> + /* If a qdisc is attached to our virtual device,
>> + * returning NETDEV_TX_BUSY is allowed.
>> + */
>> + if (qdisc_present) {
>> + rcu_read_unlock();
>> + return NETDEV_TX_BUSY;
>> + }
>> drop_reason = SKB_DROP_REASON_FULL_RING;
>> goto drop;
>> }
>>
>> /* dev->lltx requires to do our own update of trans_start */
>> - queue = netdev_get_tx_queue(dev, txq);
>> txq_trans_cond_update(queue);
>>
>> /* Notify and wake up reader process */
>> --
>> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v12 00/13] blk: honor isolcpus configuration
From: Daniel Wagner @ 2026-04-28 13:08 UTC (permalink / raw)
To: Florian Bezdeka
Cc: Aaron Tomlin, axboe, kbusch, hch, sagi, mst, aacraid,
James.Bottomley, martin.petersen, liyihang9, kashyap.desai,
sumit.saxena, shivasharan.srikanteshwara, chandrakanth.patil,
sathya.prakash, sreekanth.reddy, suganath-prabu.subramani,
ranjan.kumar, jinpu.wang, tglx, mingo, peterz, juri.lelli,
vincent.guittot, akpm, maz, ruanjinjie, bigeasy, yphbchou0911,
wagi, frederic, longman, chenridong, hare, kch, ming.lei,
tom.leiming, steve, sean, chjohnst, neelx, mproche, nick.lange,
marco.crivellari, linux-block, linux-kernel, virtualization,
linux-nvme, linux-scsi, megaraidlinux.pdl, mpi3mr-linuxdrv.pdl,
MPT-FusionLinux.pdl
In-Reply-To: <e350389a5a635660267a7a13f06529da102a95d8.camel@siemens.com>
On Mon, Apr 27, 2026 at 12:55:20PM +0200, Florian Bezdeka wrote:
> This topic reminds me of a discussion started by Tobias [1] some time
> ago about IRQ spreading of network drivers. The problem was (and still
> is) that network drivers ignore any CPU isolation when spreading out
> device IRQs.
>
> In general we have two different CPU isolation mechanisms:
> - The static one, via isolcpus= cmdline parameter
> - The dynamic one, via cgroups(v2) cpuset controller
>
> This series is only taking the static "world" into account, right? Are
> there any plans to honor the CPU isolations configured the dynamic
> way?
Dynamic configuration would require every driver to fully support
reconfiguration during runtime. Only a handful of drivers, such as
nvme-pci, are currently able to handle this.
The first task, teaching a wide range of drivers to honor CPU isolation
at boot time, is already going to be a significant amount of work.
> It has been a while since the last investigations on my end. Last time I
> went through the code, the IRQ core was completely decoupled from the
> dynamic configuration via cgroups. Are there any plans to fix that gap?
Which use case are you actually aiming to support? While dynamic
reconfiguration would be ideal, the amount of work to get there is
significant. I won't be signing up for it.
^ permalink raw reply
* [PATCH] hv_sock: fix ARM64 support
From: Hamza Mahfooz @ 2026-04-28 12:53 UTC (permalink / raw)
To: netdev
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Michael Kelley, Himadri Pandya,
linux-hyperv, virtualization, linux-kernel
VMBUS ring buffers must be page aligned. Therefore, the current value of
24K presents a challenge on ARM64 kernels (with 64K pages). So, use
VMBUS_RING_SIZE() to ensure they are always aligned and large enough to
hold all of the relevant data.
Cc: stable@vger.kernel.org
Fixes: 77ffe33363c0 ("hv_sock: use HV_HYP_PAGE_SIZE for Hyper-V communication")
Tested-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
---
net/vmw_vsock/hyperv_transport.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 069386a74557..40f09b23efa3 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -375,10 +375,10 @@ static void hvs_open_connection(struct vmbus_channel *chan)
} else {
sndbuf = max_t(int, sk->sk_sndbuf, RINGBUFFER_HVS_SND_SIZE);
sndbuf = min_t(int, sndbuf, RINGBUFFER_HVS_MAX_SIZE);
- sndbuf = ALIGN(sndbuf, HV_HYP_PAGE_SIZE);
+ sndbuf = VMBUS_RING_SIZE(sndbuf);
rcvbuf = max_t(int, sk->sk_rcvbuf, RINGBUFFER_HVS_RCV_SIZE);
rcvbuf = min_t(int, rcvbuf, RINGBUFFER_HVS_MAX_SIZE);
- rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
+ rcvbuf = VMBUS_RING_SIZE(rcvbuf);
}
chan->max_pkt_size = HVS_MAX_PKT_SIZE;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v12 05/13] blk-mq: add blk_mq_{online|possible}_queue_affinity
From: Daniel Wagner @ 2026-04-28 12:53 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Aaron Tomlin, axboe, kbusch, hch, sagi, mst, aacraid,
James.Bottomley, martin.petersen, liyihang9, kashyap.desai,
sumit.saxena, shivasharan.srikanteshwara, chandrakanth.patil,
sathya.prakash, sreekanth.reddy, suganath-prabu.subramani,
ranjan.kumar, jinpu.wang, tglx, mingo, peterz, juri.lelli,
vincent.guittot, akpm, maz, ruanjinjie, yphbchou0911, wagi,
frederic, longman, chenridong, hare, kch, ming.lei, tom.leiming,
steve, sean, chjohnst, neelx, mproche, nick.lange,
marco.crivellari, linux-block, linux-kernel, virtualization,
linux-nvme, linux-scsi, megaraidlinux.pdl, mpi3mr-linuxdrv.pdl,
MPT-FusionLinux.pdl
In-Reply-To: <20260427153416.MeVS8yxF@linutronix.de>
On Mon, Apr 27, 2026 at 05:34:16PM +0200, Sebastian Andrzej Siewior wrote:
> Which driver uses cpu_possible_mask? This mask is assigned at boot time
> once the kernel figured how many CPUs are possible based on ACPI or
> whatever the system uses. This mask does not change.
>
> I only see drivers/scsi/lpfc/lpfc_init.c using it. Looking at
> cpu_possible_mask might not be the right thing. It is usually the same
> thing as "online" except on system where ACPI thinks that something
> could be added via hotplug _or_ if the admin shuts down a CPU via
> cpuhotplug _or_ boots with less (there a command line option for
> that).
These HBAs are used on PowerPC which supports lpar (CPUs can be added
during runtime) I am told it's properly the only driver which is caring
about this type configuration, thus a bit of an odd ball.
> In case cpu_possible_mask != cpu_online_mask the intention is to
> allocate memory and setup irqs for the offline CPUs?
I can't answer this. The lpfc driver has several strategies implemented
how it spreads its resources.
^ permalink raw reply
* Re: [PATCH net-next v9 4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
From: Michael S. Tsirkin @ 2026-04-28 12:50 UTC (permalink / raw)
To: Simon Schippers
Cc: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, eperezma, leiyang, stephen, jon, tim.gebauer,
netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260428123859.19578-5-simon.schippers@tu-dortmund.de>
On Tue, Apr 28, 2026 at 02:38:59PM +0200, Simon Schippers wrote:
> This commit prevents tail-drop when a qdisc is present and the ptr_ring
> becomes full. Once an entry is successfully produced and the ptr_ring
> reaches capacity, the netdev queue is stopped instead of dropping
> subsequent packets.
>
> If producing an entry fails anyways due to a race, tun_net_xmit returns
> NETDEV_TX_BUSY, again avoiding a drop. Such races are expected because
> LLTX is enabled and the transmit path operates without the usual locking.
>
> If no qdisc is present, the previous tail-drop behavior is preserved.
>
> The existing __tun_wake_queue() function of the consumer races with the
> producer for waking/stopping the netdev queue: the consumer may drain
> the ring just as the producer stops the queue, leading to a permanent
> stall. To avoid this, the producer re-checks the ring after stopping
> and wakes the queue itself if space was just made. An
> smp_mb__after_atomic() is required so the re-peek of the ring sees any
> drain that the consumer performed.
> smp_mb__after_atomic() pairs with the test_and_clear_bit() inside of
> netif_wake_subqueue():
>
> Consumer CPU Producer CPU
> ======================== =========================
> __ptr_ring_consume()
> netif_wake_subqueue() netif_tx_stop_queue()
> /\ smp_mb__after_atomic()
> || __ptr_ring_produce_peek()
> contains RMW operation
> test_and_clear_bit()
> /\
> ||
> "Fully ordered RMW:
> smp_mb() before + after"
> - atomic_t.txt
>
> Benchmarks:
> The benchmarks show a slight regression in raw transmission performance,
> though no packets are lost anymore.
Could you include the packets received as well?
To demonstrate the gains/lack of loss.
>
> The previously introduced threshold to only wake after the queue stopped
> and half of the ring was consumed showed to be a descent choice:
> Waking the queue whenever a consume made space in the ring strongly
> degrades performance for tap, while waking only when the ring is empty
> is too late and also hurts throughput for tap & tap+vhost-net.
> Other ratios (3/4, 7/8) showed similar results (not shown here), so
> 1/2 was chosen for the sake of simplicity for both tun/tap and
> tun/tap+vhost-net.
>
> Test setup:
> AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU threads;
> Average over 50 runs @ 100,000,000 packets. SRSO and spectre v2
> mitigations disabled.
>
> Note for tap+vhost-net:
> XDP drop program active in VM -> ~2.5x faster, slower for tap due to
> more syscalls (high utilization of entry_SYSRETQ_unsafe_stack in perf)
>
> +--------------------------+--------------+----------------+----------+
> | 1 thread | Stock | Patched with | diff |
> | sending | | fq_codel qdisc | |
> +------------+-------------+--------------+----------------+----------+
> | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
> | +-------------+--------------+----------------+----------+
> | | Lost/s | 3.758 Mpps | 0 pps | |
> +------------+-------------+--------------+----------------+----------+
> | TAP | Transmitted | 3.858 Mpps | 3.816 Mpps | -1.1% |
> | +-------------+--------------+----------------+----------+
> | +vhost-net | Lost/s | 789.8 Kpps | 0 pps | |
> +------------+-------------+--------------+----------------+----------+
>
> +--------------------------+--------------+----------------+----------+
> | 2 threads | Stock | Patched with | diff |
> | sending | | fq_codel qdisc | |
> +------------+-------------+--------------+----------------+----------+
> | TAP | Transmitted | 1.117 Mpps | 1.087 Mpps | -2.7% |
> | +-------------+--------------+----------------+----------+
> | | Lost/s | 8.476 Mpps | 0 pps | |
> +------------+-------------+--------------+----------------+----------+
> | TAP | Transmitted | 3.679 Mpps | 3.464 Mpps | -5.8% |
> | +-------------+--------------+----------------+----------+
> | +vhost-net | Lost/s | 5.306 Mpps | 0 pps | |
> +------------+-------------+--------------+----------------+----------+
>
> Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
> Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
> ---
> drivers/net/tun.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index efe809597622..c2a1618cc9db 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1011,6 +1011,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> struct netdev_queue *queue;
> struct tun_file *tfile;
> int len = skb->len;
> + bool qdisc_present;
> + int ret;
>
> rcu_read_lock();
> tfile = rcu_dereference(tun->tfiles[txq]);
> @@ -1065,13 +1067,37 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>
> nf_reset_ct(skb);
>
> - if (ptr_ring_produce(&tfile->tx_ring, skb)) {
> + queue = netdev_get_tx_queue(dev, txq);
> + qdisc_present = !qdisc_txq_has_no_queue(queue);
> +
> + spin_lock(&tfile->tx_ring.producer_lock);
> + ret = __ptr_ring_produce(&tfile->tx_ring, skb);
> + if (__ptr_ring_produce_peek(&tfile->tx_ring) && qdisc_present) {
> + netif_tx_stop_queue(queue);
> + /* Re-peek and wake if the consumer drained the ring
> + * concurrently in a race. smp_mb__after_atomic() pairs
> + * with the test_and_clear_bit() of netif_wake_subqueue()
> + * in __tun_wake_queue().
> + */
> + smp_mb__after_atomic();
> + if (!__ptr_ring_produce_peek(&tfile->tx_ring))
> + netif_tx_wake_queue(queue);
> + }
> + spin_unlock(&tfile->tx_ring.producer_lock);
> +
> + if (ret) {
> + /* If a qdisc is attached to our virtual device,
> + * returning NETDEV_TX_BUSY is allowed.
> + */
> + if (qdisc_present) {
> + rcu_read_unlock();
> + return NETDEV_TX_BUSY;
> + }
> drop_reason = SKB_DROP_REASON_FULL_RING;
> goto drop;
> }
>
> /* dev->lltx requires to do our own update of trans_start */
> - queue = netdev_get_tx_queue(dev, txq);
> txq_trans_cond_update(queue);
>
> /* Notify and wake up reader process */
> --
> 2.43.0
^ permalink raw reply
* [PATCH net-next v9 1/4] tun/tap: add ptr_ring consume helper with netdev queue wakeup
From: Simon Schippers @ 2026-04-28 12:38 UTC (permalink / raw)
To: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, mst, eperezma, leiyang, stephen, jon, tim.gebauer,
simon.schippers, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260428123859.19578-1-simon.schippers@tu-dortmund.de>
Introduce tun_ring_consume() that wraps ptr_ring_consume() and calls
__tun_wake_queue(). The latter wakes the stopped netdev subqueue once
half of the ring capacity has been consumed, tracked via the new
cons_cnt field in tun_file. When the ring is empty the queue is also
woken to handle potential races. The point is to allow the queue to
be stopped when it gets full, which is required for traffic shaping -
implemented by the following "avoid ptr_ring tail-drop when a qdisc
is present".
Without the corresponding queue stopping, this patch alone causes no
regression for a tap setup sending to a qemu VM: 1.136 Mpps
to 1.141 Mpps.
Details: AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU
threads, pktgen sender; Avg over 50 runs @ 100,000,000 packets;
SRSO and spectre v2 mitigations disabled.
Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
drivers/net/tun.c | 43 +++++++++++++++++++++++++++++++++++++++----
1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index b183189f1853..e6ee2271732f 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -145,6 +145,7 @@ struct tun_file {
struct list_head next;
struct tun_struct *detached;
struct ptr_ring tx_ring;
+ int cons_cnt;
struct xdp_rxq_info xdp_rxq;
};
@@ -564,6 +565,7 @@ static void tun_queue_purge(struct tun_file *tfile)
while ((ptr = ptr_ring_consume(&tfile->tx_ring)) != NULL)
tun_ptr_free(ptr);
+ tfile->cons_cnt = 0;
skb_queue_purge(&tfile->sk.sk_write_queue);
skb_queue_purge(&tfile->sk.sk_error_queue);
}
@@ -730,6 +732,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
goto out;
}
+ tfile->cons_cnt = 0;
tfile->queue_index = tun->numqueues;
tfile->socket.sk->sk_shutdown &= ~RCV_SHUTDOWN;
@@ -2115,13 +2118,42 @@ static ssize_t tun_put_user(struct tun_struct *tun,
return total;
}
-static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
+/* Callers must hold ring.consumer_lock */
+static void __tun_wake_queue(struct tun_struct *tun, struct tun_file *tfile)
+{
+ if (__ptr_ring_empty(&tfile->tx_ring))
+ goto wake;
+
+ if (!__netif_subqueue_stopped(tun->dev, tfile->queue_index) ||
+ ++tfile->cons_cnt < tfile->tx_ring.size / 2)
+ return;
+
+wake:
+ netif_wake_subqueue(tun->dev, tfile->queue_index);
+ tfile->cons_cnt = 0;
+}
+
+static void *tun_ring_consume(struct tun_struct *tun, struct tun_file *tfile)
+{
+ void *ptr;
+
+ spin_lock(&tfile->tx_ring.consumer_lock);
+ ptr = __ptr_ring_consume(&tfile->tx_ring);
+ if (ptr)
+ __tun_wake_queue(tun, tfile);
+
+ spin_unlock(&tfile->tx_ring.consumer_lock);
+ return ptr;
+}
+
+static void *tun_ring_recv(struct tun_struct *tun, struct tun_file *tfile,
+ int noblock, int *err)
{
DECLARE_WAITQUEUE(wait, current);
void *ptr = NULL;
int error = 0;
- ptr = ptr_ring_consume(&tfile->tx_ring);
+ ptr = tun_ring_consume(tun, tfile);
if (ptr)
goto out;
if (noblock) {
@@ -2133,7 +2165,7 @@ static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
while (1) {
set_current_state(TASK_INTERRUPTIBLE);
- ptr = ptr_ring_consume(&tfile->tx_ring);
+ ptr = tun_ring_consume(tun, tfile);
if (ptr)
break;
if (signal_pending(current)) {
@@ -2170,7 +2202,7 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
if (!ptr) {
/* Read frames from ring */
- ptr = tun_ring_recv(tfile, noblock, &err);
+ ptr = tun_ring_recv(tun, tfile, noblock, &err);
if (!ptr)
return err;
}
@@ -3406,6 +3438,8 @@ static int tun_chr_open(struct inode *inode, struct file * file)
return -ENOMEM;
}
+ tfile->cons_cnt = 0;
+
mutex_init(&tfile->napi_mutex);
RCU_INIT_POINTER(tfile->tun, NULL);
tfile->flags = 0;
@@ -3614,6 +3648,7 @@ static int tun_queue_resize(struct tun_struct *tun)
for (i = 0; i < tun->numqueues; i++) {
tfile = rtnl_dereference(tun->tfiles[i]);
rings[i] = &tfile->tx_ring;
+ tfile->cons_cnt = 0;
}
list_for_each_entry(tfile, &tun->disabled, next)
rings[i++] = &tfile->tx_ring;
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v9 3/4] ptr_ring: move free-space check into separate helper
From: Simon Schippers @ 2026-04-28 12:38 UTC (permalink / raw)
To: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, mst, eperezma, leiyang, stephen, jon, tim.gebauer,
simon.schippers, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260428123859.19578-1-simon.schippers@tu-dortmund.de>
This patch moves the check for available free space for a new entry into
a separate function. As a result, __ptr_ring_produce() remains logically
unchanged, while the new helper allows callers to determine in advance
whether subsequent __ptr_ring_produce() calls will succeed. This
information can, for example, be used to temporarily stop producing until
__ptr_ring_peek() indicates that space is available again.
Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
include/linux/ptr_ring.h | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
index d2c3629bbe45..b51fe4a484be 100644
--- a/include/linux/ptr_ring.h
+++ b/include/linux/ptr_ring.h
@@ -96,6 +96,14 @@ static inline bool ptr_ring_full_bh(struct ptr_ring *r)
return ret;
}
+static inline int __ptr_ring_produce_peek(struct ptr_ring *r)
+{
+ if (unlikely(!r->size) || data_race(r->queue[r->producer]))
+ return -ENOSPC;
+
+ return 0;
+}
+
/* Note: callers invoking this in a loop must use a compiler barrier,
* for example cpu_relax(). Callers must hold producer_lock.
* Callers are responsible for making sure pointer that is being queued
@@ -103,8 +111,10 @@ static inline bool ptr_ring_full_bh(struct ptr_ring *r)
*/
static inline int __ptr_ring_produce(struct ptr_ring *r, void *ptr)
{
- if (unlikely(!r->size) || data_race(r->queue[r->producer]))
- return -ENOSPC;
+ int p = __ptr_ring_produce_peek(r);
+
+ if (p)
+ return p;
/* Make sure the pointer we are storing points to a valid data. */
/* Pairs with the dependency ordering in __ptr_ring_consume. */
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v9 4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
From: Simon Schippers @ 2026-04-28 12:38 UTC (permalink / raw)
To: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, mst, eperezma, leiyang, stephen, jon, tim.gebauer,
simon.schippers, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260428123859.19578-1-simon.schippers@tu-dortmund.de>
This commit prevents tail-drop when a qdisc is present and the ptr_ring
becomes full. Once an entry is successfully produced and the ptr_ring
reaches capacity, the netdev queue is stopped instead of dropping
subsequent packets.
If producing an entry fails anyways due to a race, tun_net_xmit returns
NETDEV_TX_BUSY, again avoiding a drop. Such races are expected because
LLTX is enabled and the transmit path operates without the usual locking.
If no qdisc is present, the previous tail-drop behavior is preserved.
The existing __tun_wake_queue() function of the consumer races with the
producer for waking/stopping the netdev queue: the consumer may drain
the ring just as the producer stops the queue, leading to a permanent
stall. To avoid this, the producer re-checks the ring after stopping
and wakes the queue itself if space was just made. An
smp_mb__after_atomic() is required so the re-peek of the ring sees any
drain that the consumer performed.
smp_mb__after_atomic() pairs with the test_and_clear_bit() inside of
netif_wake_subqueue():
Consumer CPU Producer CPU
======================== =========================
__ptr_ring_consume()
netif_wake_subqueue() netif_tx_stop_queue()
/\ smp_mb__after_atomic()
|| __ptr_ring_produce_peek()
contains RMW operation
test_and_clear_bit()
/\
||
"Fully ordered RMW:
smp_mb() before + after"
- atomic_t.txt
Benchmarks:
The benchmarks show a slight regression in raw transmission performance,
though no packets are lost anymore.
The previously introduced threshold to only wake after the queue stopped
and half of the ring was consumed showed to be a descent choice:
Waking the queue whenever a consume made space in the ring strongly
degrades performance for tap, while waking only when the ring is empty
is too late and also hurts throughput for tap & tap+vhost-net.
Other ratios (3/4, 7/8) showed similar results (not shown here), so
1/2 was chosen for the sake of simplicity for both tun/tap and
tun/tap+vhost-net.
Test setup:
AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU threads;
Average over 50 runs @ 100,000,000 packets. SRSO and spectre v2
mitigations disabled.
Note for tap+vhost-net:
XDP drop program active in VM -> ~2.5x faster, slower for tap due to
more syscalls (high utilization of entry_SYSRETQ_unsafe_stack in perf)
+--------------------------+--------------+----------------+----------+
| 1 thread | Stock | Patched with | diff |
| sending | | fq_codel qdisc | |
+------------+-------------+--------------+----------------+----------+
| TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
| +-------------+--------------+----------------+----------+
| | Lost/s | 3.758 Mpps | 0 pps | |
+------------+-------------+--------------+----------------+----------+
| TAP | Transmitted | 3.858 Mpps | 3.816 Mpps | -1.1% |
| +-------------+--------------+----------------+----------+
| +vhost-net | Lost/s | 789.8 Kpps | 0 pps | |
+------------+-------------+--------------+----------------+----------+
+--------------------------+--------------+----------------+----------+
| 2 threads | Stock | Patched with | diff |
| sending | | fq_codel qdisc | |
+------------+-------------+--------------+----------------+----------+
| TAP | Transmitted | 1.117 Mpps | 1.087 Mpps | -2.7% |
| +-------------+--------------+----------------+----------+
| | Lost/s | 8.476 Mpps | 0 pps | |
+------------+-------------+--------------+----------------+----------+
| TAP | Transmitted | 3.679 Mpps | 3.464 Mpps | -5.8% |
| +-------------+--------------+----------------+----------+
| +vhost-net | Lost/s | 5.306 Mpps | 0 pps | |
+------------+-------------+--------------+----------------+----------+
Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
drivers/net/tun.c | 30 ++++++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index efe809597622..c2a1618cc9db 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1011,6 +1011,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
struct netdev_queue *queue;
struct tun_file *tfile;
int len = skb->len;
+ bool qdisc_present;
+ int ret;
rcu_read_lock();
tfile = rcu_dereference(tun->tfiles[txq]);
@@ -1065,13 +1067,37 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
nf_reset_ct(skb);
- if (ptr_ring_produce(&tfile->tx_ring, skb)) {
+ queue = netdev_get_tx_queue(dev, txq);
+ qdisc_present = !qdisc_txq_has_no_queue(queue);
+
+ spin_lock(&tfile->tx_ring.producer_lock);
+ ret = __ptr_ring_produce(&tfile->tx_ring, skb);
+ if (__ptr_ring_produce_peek(&tfile->tx_ring) && qdisc_present) {
+ netif_tx_stop_queue(queue);
+ /* Re-peek and wake if the consumer drained the ring
+ * concurrently in a race. smp_mb__after_atomic() pairs
+ * with the test_and_clear_bit() of netif_wake_subqueue()
+ * in __tun_wake_queue().
+ */
+ smp_mb__after_atomic();
+ if (!__ptr_ring_produce_peek(&tfile->tx_ring))
+ netif_tx_wake_queue(queue);
+ }
+ spin_unlock(&tfile->tx_ring.producer_lock);
+
+ if (ret) {
+ /* If a qdisc is attached to our virtual device,
+ * returning NETDEV_TX_BUSY is allowed.
+ */
+ if (qdisc_present) {
+ rcu_read_unlock();
+ return NETDEV_TX_BUSY;
+ }
drop_reason = SKB_DROP_REASON_FULL_RING;
goto drop;
}
/* dev->lltx requires to do our own update of trans_start */
- queue = netdev_get_tx_queue(dev, txq);
txq_trans_cond_update(queue);
/* Notify and wake up reader process */
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v9 0/4] tun/tap & vhost-net: apply qdisc backpressure on full ptr_ring to reduce TX drops
From: Simon Schippers @ 2026-04-28 12:38 UTC (permalink / raw)
To: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, mst, eperezma, leiyang, stephen, jon, tim.gebauer,
simon.schippers, netdev, linux-kernel, kvm, virtualization
This patch series deals with tun/tap & vhost-net which drop incoming
SKBs whenever their internal ptr_ring buffer is full. Instead, with this
patch series, the associated netdev queue is stopped - but only when a
qdisc is attached. If no qdisc is present the existing behavior is
preserved. This patch series touches tun/tap and vhost-net, as they
share common logic and must be updated together. Modifying only one of
them would break the other.
By applying proper backpressure, this change allows the connected qdisc to
operate correctly, as reported in [1], and significantly improves
performance in real-world scenarios, as demonstrated in our paper [2]. For
example, we observed a 36% TCP throughput improvement for an OpenVPN
connection between Germany and the USA.
Synthetic pktgen benchmarks indicate a slight regression, but packet loss
no longer occurs. Pktgen benchmarks are provided per commit, with the final
commit showing the overall performance.
Thanks!
[1] Link: https://unix.stackexchange.com/questions/762935/traffic-shaping-ineffective-on-tun-device
[2] Link: https://cni.etit.tu-dortmund.de/storages/cni-etit/r/Research/Publications/2025/Gebauer_2025_VTCFall/Gebauer_VTCFall2025_AuthorsVersion.pdf
---
Changelog:
V9:
- Addressed minor nit by MST in patches 1 and 2.
- Rebased patch 3 because of commit d748047
("ptr_ring: disable KCSAN warnings").
- Documented the pair of the smp_mb__after_atomic() in tun_net_xmit()
with tun_ring_consume().
--> It simply pairs with the test_and_clear_bit() inside of
netif_wake_subqueue().
- Use 1 ptr_ring consumer spinlock instead of 2.
- Ran pktgen benchmarks with pg_set SHARED for 50 iterations on
latest kernel
--> No significant performance difference noticed
V8: https://lore.kernel.org/netdev/20260312130639.138988-1-simon.schippers@tu-dortmund.de/
- Drop code changes in drivers/net/tap.c; The code there deals with
ipvtap/macvtap which are unrelated to the goal of this patch series
and I did not realize that before
-> Greatly simplified logic, 4 instead of 9 commits
-> No more duplicated logics and distinction in vhost required
- Only wake after the queue stopped and half of the ring was consumed
as suggested by MST
-> Performance improvements for TAP, but still slightly slower
- Better benchmarking with pinned threads, XDP drop program for
tap+vhost-net and disabling CPU mitigations (and newer Ryzen 5 5600X
processor) as suggested by Jason Wang
V7: https://lore.kernel.org/netdev/20260107210448.37851-1-simon.schippers@tu-dortmund.de/
- Switch to an approach similar to veth (excluding the recently fixed
variant), as suggested by MST, with minor adjustments discussed in V6
- Rename the cover-letter title
- Add multithreaded pktgen and iperf3 benchmarks, as suggested by Jason
Wang
- Rework __ptr_ring_consume_created_space() so it can also be used after
batched consume
V6: https://lore.kernel.org/netdev/20251120152914.1127975-1-simon.schippers@tu-dortmund.de/
General:
- Major adjustments to the descriptions. Special thanks to Jon Kohler!
- Fix git bisect by moving most logic into dedicated functions and only
start using them in patch 7.
- Moved the main logic of the coupled producer and consumer into a single
patch to avoid a chicken-and-egg dependency between commits :-)
- Rebased to 6.18-rc5 and ran benchmarks again that now also include lost
packets (previously I missed a 0, so all benchmark results were higher by
factor 10...).
- Also include the benchmark in patch 7.
Producer:
- Move logic into the new helper tun_ring_produce()
- Added a smp_rmb() paired with the consumer, ensuring freed space of the
consumer is visible
- Assume that ptr_ring is not full when __ptr_ring_full_next() is called
Consumer:
- Use an unpaired smp_rmb() instead of barrier() to ensure that the
netdev_tx_queue_stopped() call completes before discarding
- Also wake the netdev queue if it was stopped before discarding and then
becomes empty
-> Fixes race with producer as identified by MST in V5
-> Waking the netdev queues upon resize is not required anymore
- Use __ptr_ring_consume_created_space() instead of messing with ptr_ring
internals
-> Batched consume now just calls
__tun_ring_consume()/__tap_ring_consume() in a loop
- Added an smp_wmb() before waking the netdev queue which is paired with
the smp_rmb() discussed above
V5: https://lore.kernel.org/netdev/20250922221553.47802-1-simon.schippers@tu-dortmund.de/T/#u
- Stop the netdev queue prior to producing the final fitting ptr_ring entry
-> Ensures the consumer has the latest netdev queue state, making it safe
to wake the queue
-> Resolves an issue in vhost-net where the netdev queue could remain
stopped despite being empty
-> For TUN/TAP, the netdev queue no longer needs to be woken in the
blocking loop
-> Introduces new helpers __ptr_ring_full_next and
__ptr_ring_will_invalidate for this purpose
- vhost-net now uses wrappers of TUN/TAP for ptr_ring consumption rather
than maintaining its own rx_ring pointer
V4: https://lore.kernel.org/netdev/20250902080957.47265-1-simon.schippers@tu-dortmund.de/T/#u
- Target net-next instead of net
- Changed to patch series instead of single patch
- Changed to new title from old title
"TUN/TAP: Improving throughput and latency by avoiding SKB drops"
- Wake netdev queue with new helpers wake_netdev_queue when there is any
spare capacity in the ptr_ring instead of waiting for it to be empty
- Use tun_file instead of tun_struct in tun_ring_recv as a more consistent
logic
- Use smp_wmb() and smp_rmb() barrier pair, which avoids any packet drops
that happened rarely before
- Use safer logic for vhost-net using RCU read locks to access TUN/TAP data
V3: https://lore.kernel.org/netdev/20250825211832.84901-1-simon.schippers@tu-dortmund.de/T/#u
- Added support for TAP and TAP+vhost-net.
V2: https://lore.kernel.org/netdev/20250811220430.14063-1-simon.schippers@tu-dortmund.de/T/#u
- Removed NETDEV_TX_BUSY return case in tun_net_xmit and removed
unnecessary netif_tx_wake_queue in tun_ring_recv.
V1: https://lore.kernel.org/netdev/20250808153721.261334-1-simon.schippers@tu-dortmund.de/T/#u
---
Simon Schippers (4):
tun/tap: add ptr_ring consume helper with netdev queue wakeup
vhost-net: wake queue of tun/tap after ptr_ring consume
ptr_ring: move free-space check into separate helper
tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
drivers/net/tun.c | 95 +++++++++++++++++++++++++++++++++++++---
drivers/vhost/net.c | 21 ++++++---
include/linux/if_tun.h | 3 ++
include/linux/ptr_ring.h | 14 +++++-
4 files changed, 119 insertions(+), 14 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH net-next v9 2/4] vhost-net: wake queue of tun/tap after ptr_ring consume
From: Simon Schippers @ 2026-04-28 12:38 UTC (permalink / raw)
To: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, mst, eperezma, leiyang, stephen, jon, tim.gebauer,
simon.schippers, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260428123859.19578-1-simon.schippers@tu-dortmund.de>
Add tun_wake_queue() to tun.c and export it for use by vhost-net. The
function validates that the file belongs to a tun/tap device,
dereferences the tun_struct under RCU, and delegates to
__tun_wake_queue().
vhost_net_buf_produce() now calls tun_wake_queue() after a successful
batched consume of the ring to allow the netdev subqueue to be woken up.
The point is to allow the queue to be stopped when it gets full, which
is required for traffic shaping - implemented by the following
"avoid ptr_ring tail-drop when a qdisc is present".
Without the corresponding queue stopping, this patch alone causes no
throughput regression for a tap+vhost-net setup sending to a qemu VM:
3.858 Mpps to 3.898 Mpps.
Details: AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU
threads, XDP drop program active in VM, pktgen sender; Avg over
50 runs @ 100,000,000 packets. SRSO and spectre v2 mitigations disabled.
Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
drivers/net/tun.c | 22 ++++++++++++++++++++++
drivers/vhost/net.c | 21 +++++++++++++++------
include/linux/if_tun.h | 3 +++
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e6ee2271732f..efe809597622 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -3765,6 +3765,28 @@ struct ptr_ring *tun_get_tx_ring(struct file *file)
}
EXPORT_SYMBOL_GPL(tun_get_tx_ring);
+/* Callers must hold ring.consumer_lock */
+void tun_wake_queue(struct file *file)
+{
+ struct tun_file *tfile;
+ struct tun_struct *tun;
+
+ if (file->f_op != &tun_fops)
+ return;
+ tfile = file->private_data;
+ if (!tfile)
+ return;
+
+ rcu_read_lock();
+
+ tun = rcu_dereference(tfile->tun);
+ if (tun)
+ __tun_wake_queue(tun, tfile);
+
+ rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(tun_wake_queue);
+
module_init(tun_init);
module_exit(tun_cleanup);
MODULE_DESCRIPTION(DRV_DESCRIPTION);
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 80965181920c..7fba518ac3cd 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -176,13 +176,21 @@ static void *vhost_net_buf_consume(struct vhost_net_buf *rxq)
return ret;
}
-static int vhost_net_buf_produce(struct vhost_net_virtqueue *nvq)
+static int vhost_net_buf_produce(struct sock *sk,
+ struct vhost_net_virtqueue *nvq)
{
+ struct file *file = sk->sk_socket->file;
struct vhost_net_buf *rxq = &nvq->rxq;
rxq->head = 0;
- rxq->tail = ptr_ring_consume_batched(nvq->rx_ring, rxq->queue,
- VHOST_NET_BATCH);
+ spin_lock(&nvq->rx_ring->consumer_lock);
+ rxq->tail = __ptr_ring_consume_batched(nvq->rx_ring, rxq->queue,
+ VHOST_NET_BATCH);
+
+ if (rxq->tail)
+ tun_wake_queue(file);
+
+ spin_unlock(&nvq->rx_ring->consumer_lock);
return rxq->tail;
}
@@ -209,14 +217,15 @@ static int vhost_net_buf_peek_len(void *ptr)
return __skb_array_len_with_tag(ptr);
}
-static int vhost_net_buf_peek(struct vhost_net_virtqueue *nvq)
+static int vhost_net_buf_peek(struct sock *sk,
+ struct vhost_net_virtqueue *nvq)
{
struct vhost_net_buf *rxq = &nvq->rxq;
if (!vhost_net_buf_is_empty(rxq))
goto out;
- if (!vhost_net_buf_produce(nvq))
+ if (!vhost_net_buf_produce(sk, nvq))
return 0;
out:
@@ -995,7 +1004,7 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
unsigned long flags;
if (rvq->rx_ring)
- return vhost_net_buf_peek(rvq);
+ return vhost_net_buf_peek(sk, rvq);
spin_lock_irqsave(&sk->sk_receive_queue.lock, flags);
head = skb_peek(&sk->sk_receive_queue);
diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
index 80166eb62f41..ab3b4ebca059 100644
--- a/include/linux/if_tun.h
+++ b/include/linux/if_tun.h
@@ -22,6 +22,7 @@ struct tun_msg_ctl {
#if defined(CONFIG_TUN) || defined(CONFIG_TUN_MODULE)
struct socket *tun_get_socket(struct file *);
struct ptr_ring *tun_get_tx_ring(struct file *file);
+void tun_wake_queue(struct file *file);
static inline bool tun_is_xdp_frame(void *ptr)
{
@@ -55,6 +56,8 @@ static inline struct ptr_ring *tun_get_tx_ring(struct file *f)
return ERR_PTR(-EINVAL);
}
+static inline void tun_wake_queue(struct file *f) {}
+
static inline bool tun_is_xdp_frame(void *ptr)
{
return false;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] hv_sock: fix ARM64 support
From: Hamza Mahfooz @ 2026-04-28 12:35 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Long Li, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Michael Kelley, Himadri Pandya,
linux-hyperv, virtualization, linux-kernel, stable
In-Reply-To: <afCjpGa4Xm4hIPj6@sgarzare-redhat>
On Tue, Apr 28, 2026 at 02:15:26PM +0200, Stefano Garzarella wrote:
> On Tue, Apr 28, 2026 at 07:05:30AM -0400, Hamza Mahfooz wrote:
> > VMBUS ring buffers must be page aligned. Therefore, the current value of
> > 24K presents a challenge on ARM64 kernels (with 64K pages). So, use
> > VMBUS_RING_SIZE() to ensure they are always aligned and large enough to
> > hold all of the relevant data.
> >
> > Cc: stable@kernel.vger.org
>
> mmm, this is the first time I've seen this address used for stable. Even
> after searching in the log, I don't see anyone else who's used it.
> Where did you get it from?
>
> From Documentation/process/stable-kernel-rules.rst :
>
> To have a patch you submit for mainline inclusion later automatically
> picked up for stable trees, add this tag in the sign-off area::
>
> Cc: stable@vger.kernel.org
Ya, that is the address I intended to Cc, not sure how I ended up
mangling it though.
>
>
> The patch LGTM.
>
> Thanks,
> Stefano
>
> > Fixes: 77ffe33363c0 ("hv_sock: use HV_HYP_PAGE_SIZE for Hyper-V communication")
> > Tested-by: Dexuan Cui <decui@microsoft.com>
> > Reviewed-by: Dexuan Cui <decui@microsoft.com>
> > Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
> > ---
> > net/vmw_vsock/hyperv_transport.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> > index 069386a74557..40f09b23efa3 100644
> > --- a/net/vmw_vsock/hyperv_transport.c
> > +++ b/net/vmw_vsock/hyperv_transport.c
> > @@ -375,10 +375,10 @@ static void hvs_open_connection(struct vmbus_channel *chan)
> > } else {
> > sndbuf = max_t(int, sk->sk_sndbuf, RINGBUFFER_HVS_SND_SIZE);
> > sndbuf = min_t(int, sndbuf, RINGBUFFER_HVS_MAX_SIZE);
> > - sndbuf = ALIGN(sndbuf, HV_HYP_PAGE_SIZE);
> > + sndbuf = VMBUS_RING_SIZE(sndbuf);
> > rcvbuf = max_t(int, sk->sk_rcvbuf, RINGBUFFER_HVS_RCV_SIZE);
> > rcvbuf = min_t(int, rcvbuf, RINGBUFFER_HVS_MAX_SIZE);
> > - rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
> > + rcvbuf = VMBUS_RING_SIZE(rcvbuf);
> > }
> >
> > chan->max_pkt_size = HVS_MAX_PKT_SIZE;
> > --
> > 2.54.0
> >
>
^ permalink raw reply
* Re: [PATCH] hv_sock: fix ARM64 support
From: Stefano Garzarella @ 2026-04-28 12:15 UTC (permalink / raw)
To: Hamza Mahfooz
Cc: netdev, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Long Li, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Michael Kelley, Himadri Pandya,
linux-hyperv, virtualization, linux-kernel, stable
In-Reply-To: <20260428110530.1717647-1-hamzamahfooz@linux.microsoft.com>
On Tue, Apr 28, 2026 at 07:05:30AM -0400, Hamza Mahfooz wrote:
>VMBUS ring buffers must be page aligned. Therefore, the current value of
>24K presents a challenge on ARM64 kernels (with 64K pages). So, use
>VMBUS_RING_SIZE() to ensure they are always aligned and large enough to
>hold all of the relevant data.
>
>Cc: stable@kernel.vger.org
mmm, this is the first time I've seen this address used for stable. Even
after searching in the log, I don't see anyone else who's used it.
Where did you get it from?
From Documentation/process/stable-kernel-rules.rst :
To have a patch you submit for mainline inclusion later automatically
picked up for stable trees, add this tag in the sign-off area::
Cc: stable@vger.kernel.org
The patch LGTM.
Thanks,
Stefano
>Fixes: 77ffe33363c0 ("hv_sock: use HV_HYP_PAGE_SIZE for Hyper-V communication")
>Tested-by: Dexuan Cui <decui@microsoft.com>
>Reviewed-by: Dexuan Cui <decui@microsoft.com>
>Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
>---
> net/vmw_vsock/hyperv_transport.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 069386a74557..40f09b23efa3 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -375,10 +375,10 @@ static void hvs_open_connection(struct vmbus_channel *chan)
> } else {
> sndbuf = max_t(int, sk->sk_sndbuf, RINGBUFFER_HVS_SND_SIZE);
> sndbuf = min_t(int, sndbuf, RINGBUFFER_HVS_MAX_SIZE);
>- sndbuf = ALIGN(sndbuf, HV_HYP_PAGE_SIZE);
>+ sndbuf = VMBUS_RING_SIZE(sndbuf);
> rcvbuf = max_t(int, sk->sk_rcvbuf, RINGBUFFER_HVS_RCV_SIZE);
> rcvbuf = min_t(int, rcvbuf, RINGBUFFER_HVS_MAX_SIZE);
>- rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
>+ rcvbuf = VMBUS_RING_SIZE(rcvbuf);
> }
>
> chan->max_pkt_size = HVS_MAX_PKT_SIZE;
>--
>2.54.0
>
^ permalink raw reply
* [PATCH] hv_sock: fix ARM64 support
From: Hamza Mahfooz @ 2026-04-28 11:05 UTC (permalink / raw)
To: netdev
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Michael Kelley, Himadri Pandya,
linux-hyperv, virtualization, linux-kernel, Hamza Mahfooz, stable
VMBUS ring buffers must be page aligned. Therefore, the current value of
24K presents a challenge on ARM64 kernels (with 64K pages). So, use
VMBUS_RING_SIZE() to ensure they are always aligned and large enough to
hold all of the relevant data.
Cc: stable@kernel.vger.org
Fixes: 77ffe33363c0 ("hv_sock: use HV_HYP_PAGE_SIZE for Hyper-V communication")
Tested-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
---
net/vmw_vsock/hyperv_transport.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 069386a74557..40f09b23efa3 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -375,10 +375,10 @@ static void hvs_open_connection(struct vmbus_channel *chan)
} else {
sndbuf = max_t(int, sk->sk_sndbuf, RINGBUFFER_HVS_SND_SIZE);
sndbuf = min_t(int, sndbuf, RINGBUFFER_HVS_MAX_SIZE);
- sndbuf = ALIGN(sndbuf, HV_HYP_PAGE_SIZE);
+ sndbuf = VMBUS_RING_SIZE(sndbuf);
rcvbuf = max_t(int, sk->sk_rcvbuf, RINGBUFFER_HVS_RCV_SIZE);
rcvbuf = min_t(int, rcvbuf, RINGBUFFER_HVS_MAX_SIZE);
- rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
+ rcvbuf = VMBUS_RING_SIZE(rcvbuf);
}
chan->max_pkt_size = HVS_MAX_PKT_SIZE;
--
2.54.0
^ permalink raw reply related
* [PATCH RFC 09/11] x86/msr: Add macros for preparing to switch rdmsr/wrmsr interfaces
From: Juergen Gross @ 2026-04-28 10:42 UTC (permalink / raw)
To: linux-kernel, x86, virtualization
Cc: Juergen Gross, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list
In-Reply-To: <20260428104205.916924-1-jgross@suse.com>
In order to prepare switching the rdmsr(), rdmasr_safe(), wrmsr() and
wrmsr_safe() interfaces to 64-bit values instead of 32-bit pairs, add
macros to call different implementations depending on the number of
passed parameters.
This enables to use the same function/macro names as today while
doing the interface switch per component instead of one go.
At the same time switch the rdmsr related interfaces to inline
functions, avoiding to change variables passed as a parameter to be
changed.
The helper macros will be removed when all users of the current
interfaces have been switched to the new ones.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
arch/x86/include/asm/msr.h | 46 +++++++++++++++++++++++++++++----
arch/x86/include/asm/paravirt.h | 6 ++---
2 files changed, 44 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index a5596d268053..4dd181aedb00 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -12,6 +12,7 @@
#include <uapi/asm/msr.h>
#include <asm/shared/msr.h>
+#include <linux/args.h>
#include <linux/types.h>
#include <linux/percpu.h>
@@ -179,14 +180,14 @@ static inline u64 native_read_pmc(int counter)
* pointer indirection), this allows gcc to optimize better
*/
-#define rdmsr(msr, low, high) \
+#define __rdmsr_3(msr, low, high) \
do { \
u64 __val = native_read_msr((msr)); \
(void)((low) = (u32)__val); \
(void)((high) = (u32)(__val >> 32)); \
} while (0)
-static inline void wrmsr(u32 msr, u32 low, u32 high)
+static inline void __wrmsr_3(u32 msr, u32 low, u32 high)
{
native_write_msr(msr, (u64)high << 32 | low);
}
@@ -206,7 +207,7 @@ static inline int wrmsrq_safe(u32 msr, u64 val)
}
/* rdmsr with exception handling */
-#define rdmsr_safe(msr, low, high) \
+#define __rdmsr_safe_3(msr, low, high) \
({ \
u64 __val; \
int __err = native_read_msr_safe((msr), &__val); \
@@ -243,13 +244,48 @@ static __always_inline void wrmsrns(u32 msr, u64 val)
}
/*
- * Dual u32 version of wrmsrq_safe():
+ * Dual u32 versions of wrmsr_safe():
*/
-static inline int wrmsr_safe(u32 msr, u32 low, u32 high)
+static __always_inline int __wrmsr_safe_3(u32 msr, u32 low, u32 high)
{
return wrmsrq_safe(msr, (u64)high << 32 | low);
}
+/*
+ * u64 versions of rdmsr/wrmsr[_safe]():
+ */
+static __always_inline u64 __rdmsr_1(u32 msr)
+{
+ u64 val;
+
+ rdmsrq(msr, val);
+
+ return val;
+}
+
+static __always_inline void __wrmsr_2(u32 msr, u64 val)
+{
+ wrmsrq(msr, val);
+}
+
+static __always_inline int __rdmsr_safe_2(u32 msr, u64 *p)
+{
+ return rdmsrq_safe(msr, p);
+}
+
+static __always_inline int __wrmsr_safe_2(u32 msr, u64 val)
+{
+ return wrmsrq_safe(msr, val);
+}
+
+/*
+ * Macros for selecting u64 or dual u32 versions of rdmsr/wrmsr[_safe]():
+ */
+#define rdmsr(...) CONCATENATE(__rdmsr_, COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__)
+#define wrmsr(...) CONCATENATE(__wrmsr_, COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__)
+#define rdmsr_safe(...) CONCATENATE(__rdmsr_safe_, COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__)
+#define wrmsr_safe(...) CONCATENATE(__wrmsr_safe_, COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__)
+
struct msr __percpu *msrs_alloc(void);
void msrs_free(struct msr __percpu *msrs);
int msr_set_bit(u32 msr, u8 bit);
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index cdfe4007443e..359fbc09f132 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -150,14 +150,14 @@ static inline int paravirt_write_msr_safe(u32 msr, u64 val)
return PVOP_CALL2(int, pv_ops, cpu.write_msr_safe, msr, val);
}
-#define rdmsr(msr, val1, val2) \
+#define __rdmsr_3(msr, val1, val2) \
do { \
u64 _l = paravirt_read_msr(msr); \
val1 = (u32)_l; \
val2 = _l >> 32; \
} while (0)
-static __always_inline void wrmsr(u32 msr, u32 low, u32 high)
+static __always_inline void __wrmsr_3(u32 msr, u32 low, u32 high)
{
paravirt_write_msr(msr, (u64)high << 32 | low);
}
@@ -178,7 +178,7 @@ static inline int wrmsrq_safe(u32 msr, u64 val)
}
/* rdmsr with exception handling */
-#define rdmsr_safe(msr, a, b) \
+#define __rdmsr_safe_3(msr, a, b) \
({ \
u64 _l; \
int _err = paravirt_read_msr_safe((msr), &_l); \
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0] scsi: virtio_scsi: Move INIT_WORK calls to virtscsi_probe()
From: Sasha Levin @ 2026-04-28 10:40 UTC (permalink / raw)
To: patches, stable
Cc: Joshua Daley, Stefan Hajnoczi, Martin K. Petersen, Sasha Levin,
mst, jasowang, jejb, virtualization, linux-scsi, linux-kernel
In-Reply-To: <20260428104133.2858589-1-sashal@kernel.org>
From: Joshua Daley <jdaley@linux.ibm.com>
[ Upstream commit da3159a3b3fdc05c6bdba2fd4f4802a6718d879a ]
The last step of virtscsi_handle_event() is to call
virtscsi_kick_event(), which calls INIT_WORK on its own work
item. INIT_WORK resets the work item's data bits to 0.
If this occurs while the work item is being flushed by
cancel_work_sync(), then kernel/workqueue.c/work_offqd_enable triggers a
kernel warning, as it expects the "disable" bit to be 1:
[ 21.450115] workqueue: work disable count underflowed
[ 21.450117] WARNING: CPU: 1 PID: 56 at kernel/workqueue.c:4328 enable_work+0x10a/0x120
...
[ 21.450171] Call Trace:
[ 21.450173] [<000003db2e5bdc3e>] enable_work+0x10e/0x120
[ 21.450176] ([<000003db2e5bdc3a>] enable_work+0x10a/0x120)
[ 21.450178] [<000003db2e5bdd86>] cancel_work_sync+0x86/0xa0
[ 21.450181] [<000003daae97d9e4>] virtscsi_remove+0xb4/0xd0 [virtio_scsi]
[ 21.450184] [<000003db2ef3b5ca>] virtio_dev_remove+0x6a/0xd0
[ 21.450186] [<000003db2ef9106c>] device_release_driver_internal+0x1ac/0x260
[ 21.450190] [<000003db2ef8edc8>] bus_remove_device+0xf8/0x190
[ 21.450192] [<000003db2ef88d72>] device_del+0x142/0x340
[ 21.450194] [<000003db2ef88fa0>] device_unregister+0x30/0xa0
[ 21.450196] [<000003db2ef3b2fa>] unregister_virtio_device+0x2a/0x40
This warning may occur if a controller is detached immediately following
a disk detach.
Move the INIT_WORK call to prevent this. Don't re-init event list work
items in virtscsi_kick_event(), init them only once in virtscsi_probe()
instead.
Signed-off-by: Joshua Daley <jdaley@linux.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Link: https://patch.msgid.link/20260325180857.3675854-2-jdaley@linux.ibm.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have enough information to complete the analysis.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject Line**
- Record: `scsi: virtio_scsi` "Move INIT_WORK calls to virtscsi_probe()"
— restructures INIT_WORK placement to avoid race with
`cancel_work_sync()` during controller removal.
**Step 1.2: Tags**
- Record:
- `Signed-off-by: Joshua Daley <jdaley@linux.ibm.com>` (author)
- `Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>` (subsystem
expert)
- `Link: https://patch.msgid.link/20260325180857.3675854-2-
jdaley@linux.ibm.com` (mailing list)
- `Signed-off-by: Martin K. Petersen` (SCSI maintainer, applied the
patch)
- No `Fixes:` tag, no `Cc: stable` (expected - this is why review is
needed)
- No `Reported-by:` but reproduction steps present in cover letter
**Step 1.3: Commit Body Analysis**
- Record: The commit describes a race where:
- `virtscsi_handle_event()` (work function) calls
`virtscsi_kick_event()` at the end
- `virtscsi_kick_event()` calls `INIT_WORK` on the SAME work item that
is currently executing
- `INIT_WORK` resets work->data bits (including the workqueue disable
count) to 0
- If this happens while `cancel_work_sync()` is flushing the work,
`work_offqd_enable` sees the disable count was cleared and triggers
"work disable count underflowed" WARN
- Includes a full stack trace on S390; trigger: "controller is
detached immediately following a disk detach"
**Step 1.4: Hidden Bug Fix Detection**
- Record: Not hidden - clearly labeled as fixing a warning. Race
condition fix disguised as "Move INIT_WORK".
## PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- Record: 1 file (`drivers/scsi/virtio_scsi.c`), ~5 net lines added.
Changed functions: `virtscsi_kick_event()` (INIT_WORK removed) and
`virtscsi_probe()` (INIT_WORK loop added). Single-file surgical fix.
**Step 2.2: Code Flow**
- Record:
- Before: `INIT_WORK(&event_node->work, virtscsi_handle_event)` called
in `virtscsi_kick_event()`, which is invoked from both
`virtscsi_kick_event_all()` (at probe/restore time) AND from
`virtscsi_handle_event()` itself (re-queueing at end of event
handling).
- After: `INIT_WORK` called once in `virtscsi_probe()` inside a `for`
loop over all 8 event_list entries (guarded by
VIRTIO_SCSI_F_HOTPLUG). `virtscsi_kick_event()` no longer resets the
work struct state.
- Forward declaration of `virtscsi_handle_event` removed (probe is
after the definition).
**Step 2.3: Bug Mechanism**
- Record: **Race condition fix** (category b from playbook). The issue
is that `INIT_WORK` resets all state bits in `work->data` (including
the disable count introduced in v6.10 by commit `86898fa6b8cd9`).
Internally, `cancel_work_sync()` now calls `__cancel_work_sync(work,
0)` → `__cancel_work(work, WORK_CANCEL_DISABLE)` which increments the
disable count via `work_offqd_disable()`, then `__flush_work()` waits
for the function to complete, then calls `enable_work()` to decrement.
If the work function calls `INIT_WORK` during the flush, disable count
goes 1→0; later `enable_work()` sees 0 and triggers `WARN_ONCE(true,
"workqueue: work disable count underflowed\n")` at
`kernel/workqueue.c:4422`.
**Step 2.4: Fix Quality**
- Record: Obviously correct. The INIT_WORK was redundant after the first
call (work's function pointer doesn't change between kicks). Moving it
to probe() eliminates the race. Low regression risk: the work struct
state is preserved across kicks (no need to re-init), and it persists
through freeze/resume cycles (virtscsi_freeze doesn't cancel work, so
state remains intact).
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: Blame of Buggy Code**
- Record: `INIT_WORK(&event_node->work, virtscsi_handle_event)` in
`virtscsi_kick_event()` was introduced by commit `365a715009411`
"[SCSI] virtio-scsi: hotplug support for virtio-scsi" (v3.6-rc1,
2012). The pattern has existed unchanged for 13+ years in all stable
trees.
**Step 3.2: No Fixes: Tag to Follow**
- Record: No Fixes: tag present. The WARN symptom was enabled by commit
`86898fa6b8cd9` "workqueue: Implement disable/enable for (delayed)
work items" which landed in **v6.10-rc1**. Before v6.10 the same race
existed but did not trigger this specific WARN (cancel_work_sync
didn't use the disable count).
**Step 3.3: File History**
- Record: Recent virtio_scsi.c history shows a related commit
`2678369e8efe0` "virtio_scsi: fix DMA cacheline issues for events" (by
Michael Tsirkin, Dec 2025) which restructured the event buffers. The
currently analyzed patch applies cleanly on top of that. No patch
dependencies required beyond the usual.
**Step 3.4: Author Context**
- Record: Joshua Daley (IBM); this is their first virtio_scsi fix.
However, the patch was Reviewed-by Stefan Hajnoczi (original virtio-
scsi author at IBM/RedHat and primary reviewer for virtio_scsi), and
applied by Martin K. Petersen (SCSI maintainer).
**Step 3.5: Dependencies**
- Record: Standalone fix. A second patch in the series (2/2 "kick
event_list unconditionally") is independent and addresses a different
cleanup - not required for this one to work. This patch doesn't depend
on the other.
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: Patch Discussion**
- Record: Retrieved full thread via `b4 mbox
20260325180857.3675854-2-jdaley@linux.ibm.com`. Series is at v4.
Previous versions (v1-v3) had different approaches (INIT_WORK moved to
`virtscsi_init()` initially). Changelog notes v4 addresses bisection
concerns (by placing this patch first in the series) and
suspend/resume concerns (by choosing `virtscsi_probe()` rather than
`virtscsi_init()`). **No stable nomination in the thread**, but the
patch is clearly framed as a bug fix.
**Step 4.2: Reviewers**
- Record: Cc'd: linux-scsi, linux-kernel, virtualization list, MST,
jasowang, pbonzini (QEMU/virtio maintainers), stefanha (virtio-scsi
expert), eperezma, Martin Petersen (SCSI maintainer), and multiple IBM
S390 engineers (mjrosato, farman, frankja). Stefan Hajnoczi's
Reviewed-by tag confirms subsystem expert review.
**Step 4.3: Bug Report**
- Record: No syzbot report. The reporter is the author himself running
tests on IBM S390 (evidenced by addresses in stack trace
`000003db2e5...`). The cover letter documents that the warning is
reliably reproducible by adding `msleep(1000)` before INIT_WORK and
running `virsh detach-device disk; virsh detach-device controller`.
**Step 4.4: Related Patches**
- Record: The series "scsi: virtio_scsi: move INIT_WORK calls to
virtscsi_probe" contains 2 patches, both applied by Martin K. Petersen
to `7.1/scsi-queue` (`[1/2] da3159a3b3fd` and `[2/2] 0019a3a5756b`).
**Step 4.5: Stable-specific Discussion**
- Record: No explicit stable discussion in the thread. The v4 changelog
does not mention stable.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: Key Functions**
- Record: `virtscsi_kick_event` (INIT_WORK removed), `virtscsi_probe`
(INIT_WORK loop added), `virtscsi_handle_event` (forward declaration
removed since probe is below it).
**Step 5.2: Callers of `virtscsi_kick_event`**
- Record: `virtscsi_kick_event_all()` (called at probe and restore) and
`virtscsi_handle_event()` (the work function itself, for re-queueing).
`virtscsi_kick_event_all` is called from `virtscsi_probe()` and
`virtscsi_restore()`.
**Step 5.3: Callees**
- Record: `virtscsi_kick_event` calls `sg_init_one`,
`virtqueue_add_inbuf_cache_clean`, `virtqueue_kick`. None of these
interact with work struct state.
**Step 5.4: Reachability**
- Record: The race path is reachable from userspace via standard device
hotplug operations (virsh detach-device or equivalent QEMU API calls).
Very common in cloud/virt environments.
**Step 5.5: Similar Patterns**
- Record: The anti-pattern of "calling INIT_WORK from within the work
function on its own work_struct" is known to be racy with
cancel_work_sync. This is why v6.10+ workqueue added the WARN to
detect it.
## PHASE 6: STABLE TREE ANALYSIS
**Step 6.1: Code in Stable Trees**
- Record: Verified by reading
`remotes/stable/linux-6.6.y:drivers/scsi/virtio_scsi.c` and
`linux-6.12.y` — both have the exact same
`INIT_WORK(&event_node->work, virtscsi_handle_event)` pattern in
`virtscsi_kick_event()` and the same
`virtscsi_probe()`/`virtscsi_remove()` structure. Code exists
unchanged in all maintained stable trees (back to at least 5.15).
**Step 6.2: Backport Difficulty**
- Record: The patch should apply cleanly or with trivial adjustments.
The surrounding code in `virtscsi_probe()` is similar across stable
trees, though there was a recent reorganization (`2678369e8efe0`
"virtio_scsi: fix DMA cacheline issues for events" in mainline, not in
stable). In 6.12.y, `event_node->event` is still an inline struct (not
a pointer); the patch's INIT_WORK change is independent of that.
**Step 6.3: Related Fixes in Stable**
- Record: No prior fix for this race in stable. The WARN_ONCE at
kernel/workqueue.c:4422 was introduced in v6.10 (commit
`86898fa6b8cd9`).
## PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1: Subsystem**
- Record: `drivers/scsi/virtio_scsi.c` — virtio-scsi driver.
Criticality: **IMPORTANT**. Used by essentially every KVM/QEMU-based
virtualization stack (including cloud providers using KVM, libvirt,
AWS EC2, GCP GCE, OpenStack).
**Step 7.2: Subsystem Activity**
- Record: Moderately active (~20 commits in recent history, many
cleanup/refactoring). Core logic unchanged since v3.6.
## PHASE 8: IMPACT AND RISK
**Step 8.1: Affected Users**
- Record: All users of virtio-scsi on v6.10+ kernels who perform hotplug
operations (disk/controller detach). This is a massive user base in
virtualization.
**Step 8.2: Trigger Conditions**
- Record: Normal administrative workflow: detach a disk, then detach the
controller immediately. Reproducible with standard virsh commands. Not
privileged-user-triggerable from guest, but a host-side operation.
**Step 8.3: Failure Mode**
- Record: Kernel `WARN_ONCE` with stack trace in dmesg. **Severity:
MEDIUM**. Not a crash, not data corruption, not security-relevant.
But: user-visible warning that could trigger monitoring alerts, CI
failures (many CI systems treat kernel WARN as failure), and user
concern. The pre-v6.10 race still exists but without the visible WARN
— potentially other subtle effects but not documented.
**Step 8.4: Risk vs Benefit**
- Record:
- BENEFIT: Eliminates a user-visible kernel warning in a very common
virtualization path. Moderate benefit.
- RISK: Very low — 5 net lines, moves a single INIT_WORK call,
reviewed by subsystem expert, applied by maintainer. The work struct
state is correctly preserved across freeze/resume cycles.
- Ratio: Favorable for backporting.
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: Evidence Summary**
FOR backporting:
- Real race condition with reproducible user-visible WARN
- Small, surgical fix (5 net lines)
- Reviewed by subsystem expert (Stefan Hajnoczi)
- Applied by SCSI maintainer (Martin K. Petersen)
- Code pattern identical in all stable trees v6.10+ where the WARN is
triggered
- Common workflow (VM hotplug) triggers this bug
- Low regression risk
AGAINST backporting:
- Symptom is WARN, not crash/corruption
- No explicit Cc: stable nomination
- No Fixes: tag
- Race has existed since v3.6 without major user impact reports
**Step 9.2: Stable Rules**
1. Obviously correct: YES — trivially correct (just relocates INIT_WORK
to run once)
2. Fixes real bug: YES — triggers WARN in v6.10+
3. Important issue: MEDIUM (WARN, not crash)
4. Small and contained: YES (5 lines, one file)
5. No new features: YES
6. Applies cleanly: YES for recent stable trees
**Step 9.3: Exception Categories**
- Record: Not an exception category (not device ID, quirk, DT, build, or
doc fix). Regular bug fix.
**Step 9.4: Decision**
The fix addresses a reproducible race condition with a clear user-
visible symptom on v6.10+ kernels (6.12, 6.17, 6.18, 6.19 stable trees).
It has been reviewed by the virtio-scsi expert, applied by the SCSI
maintainer, and affects a widely-deployed driver (used in virtually all
KVM/QEMU VMs). The patch is minimal, correct, and low-risk. The symptom
is only a WARN (not a crash), but it occurs in common hotplug workflows
and the fix is safe.
---
## Verification
- [Phase 1] Parsed tags: Reviewed-by Stefan Hajnoczi, Link to lore,
Signed-off-by Martin K. Petersen; no Fixes/Cc stable tags
- [Phase 2] Diff analysis: 5 net lines changed in
`drivers/scsi/virtio_scsi.c`; INIT_WORK moved from
`virtscsi_kick_event` to a loop in `virtscsi_probe` guarded by
VIRTIO_SCSI_F_HOTPLUG
- [Phase 3] `git log -S"INIT_WORK(&event_node->work"`: pattern
introduced by `365a715009411` in v3.6-rc1 (2012)
- [Phase 3] `git describe --contains 365a715009411`: v3.6-rc1, confirmed
pattern has been stable for 13+ years
- [Phase 3] `git log -S"work disable count underflowed"`: WARN
introduced by `86898fa6b8cd9` in v6.10-rc1 — this is why the visible
symptom only exists v6.10+
- [Phase 3] Read kernel/workqueue.c lines 4407-4499 to verify
`enable_work()`/`__cancel_work_sync()` logic and confirm the race
mechanism
- [Phase 3] Read include/linux/workqueue.h: confirmed `INIT_WORK` →
`__INIT_WORK_KEY` → resets `work->data = WORK_DATA_INIT()` (all bits
zeroed, clobbering disable count)
- [Phase 4] `b4 mbox 20260325180857.3675854-2-jdaley@linux.ibm.com`:
retrieved full thread (7 messages, 2 reviewers, 1 apply confirmation)
- [Phase 4] Thread content confirmed: v4 is latest, Reviewed-by tag
added, Martin K. Petersen applied to 7.1/scsi-queue
(git.kernel.org/mkp/scsi/c/da3159a3b3fd)
- [Phase 4] No explicit stable nomination in the mailing list discussion
- [Phase 5] `grep "virtscsi_kick_event\b"`: 3 hits — function
definition, call from `virtscsi_kick_event_all()` (probe/restore),
call from `virtscsi_handle_event()` (re-queue after event processing)
- [Phase 6] Read `remotes/stable/linux-6.6.y:drivers/scsi/virtio_scsi.c`
and `linux-6.12.y`: confirmed identical `INIT_WORK(&event_node->work,
...)` pattern in `virtscsi_kick_event()`, so fix applies conceptually
cleanly
- [Phase 6] Confirmed via search that recent commit `2678369e8efe0`
"virtio_scsi: fix DMA cacheline issues for events" changed the event
buffer layout in mainline but doesn't affect the INIT_WORK move
- [Phase 7] Subsystem: virtio-scsi — widely used in all KVM/QEMU
virtualization
- [Phase 8] Failure mode: `WARN_ONCE` at kernel/workqueue.c:4422, stack
trace visible in dmesg; severity MEDIUM (not a
crash/corruption/security)
- UNVERIFIED: Whether the pre-v6.10 race has any user-visible effect
beyond the post-v6.10 WARN. The fix is still logically correct for
pre-v6.10 but the symptom documentation only covers v6.10+.
- UNVERIFIED: Whether the fix requires any adjustment for stable trees
(the diff in the thread differs slightly from the presented diff —
presented diff has a few whitespace/context differences from the v4
mailed patch, but the functional change is identical)
The commit is a small, well-reviewed bug fix for a reproducible race
condition that triggers a kernel WARN in a widely-deployed driver
(virtio-scsi used in essentially all KVM/QEMU VMs) on v6.10+ kernels.
The fix is minimal (5 net lines, one file), obviously correct (the work
struct doesn't need to be re-initialized on every kick), reviewed by the
subsystem expert (Stefan Hajnoczi), and applied by the SCSI maintainer.
Regression risk is very low.
**YES**
drivers/scsi/virtio_scsi.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 0ed8558dad724..64b6c942f5720 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -233,7 +233,6 @@ static void virtscsi_ctrl_done(struct virtqueue *vq)
virtscsi_vq_done(vscsi, &vscsi->ctrl_vq, virtscsi_complete_free);
};
-static void virtscsi_handle_event(struct work_struct *work);
static int virtscsi_kick_event(struct virtio_scsi *vscsi,
struct virtio_scsi_event_node *event_node)
@@ -242,7 +241,6 @@ static int virtscsi_kick_event(struct virtio_scsi *vscsi,
struct scatterlist sg;
unsigned long flags;
- INIT_WORK(&event_node->work, virtscsi_handle_event);
sg_init_one(&sg, event_node->event, sizeof(struct virtio_scsi_event));
spin_lock_irqsave(&vscsi->event_vq.vq_lock, flags);
@@ -984,8 +982,11 @@ static int virtscsi_probe(struct virtio_device *vdev)
virtio_device_ready(vdev);
- if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
+ if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
+ for (int i = 0; i < VIRTIO_SCSI_EVENT_LEN; i++)
+ INIT_WORK(&vscsi->event_list[i].work, virtscsi_handle_event);
virtscsi_kick_event_all(vscsi);
+ }
scsi_scan_host(shost);
return 0;
--
2.53.0
^ permalink raw reply related
* [PATCH RFC 00/11] x86/msr: Reduce MSR access interfaces
From: Juergen Gross @ 2026-04-28 10:41 UTC (permalink / raw)
To: linux-kernel, x86, linux-edac, linux-pm, linux-hwmon,
linux-perf-users, platform-driver-x86, linux-acpi, virtualization
Cc: Juergen Gross, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Tony Luck, Rafael J. Wysocki,
Viresh Kumar, Guenter Roeck, Daniel Lezcano, Zhang Rui,
Lukasz Luba, Peter Zijlstra, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Huang Rui,
Mario Limonciello, Perry Yuan, K Prateek Nayak,
Srinivas Pandruvada, Len Brown, Hans de Goede, Ilpo Järvinen,
Ajay Kaher, Alexey Makhalov, Broadcom internal kernel review list
After my first attempt to rework the MSR access functions [1] this is
the result of the feedback I got.
I have still followed the idea to:
- Reduce the number of MSR access functions by keeping the ones with
64-bit values only (instead of the dual 32-bit ones).
- Try to have inline functions instead of macros for rdmsr*(), removing
the hard to read cases where parameters specified the variables for
the results.
One feedback I got was NOT to rename the access functions, which I
avoided in my new approach.
The first 8 patches are a complete set for achieving especially the
first point above for the *_on_cpu() functions.
Patch 9 is preparing the switch of the CPU-local MSR access functions
to have only rdmsr(), rdmsr_safe(), wrmsr() and wrmsr_safe() (all with
64-bit values and as inline functions) in the end. For this purpose
the already existing functions/macros are overloaded via macros to
accept both variants (64-bit and dual 32-bit values) during the phase
switching the different subsystems to the new scheme. This has the
advantage to avoid having to either patch all users of the current
functions in one patch (like done in the first 8 patches), or having
to use intermediate function names with need to be patched at the end
another time. The resulting patches would be very hard to review due
to their size.
The last 2 patches are examples how switches of subsystems would look
like.
Up to now all of that is compile tested only.
[1]: https://lore.kernel.org/lkml/20260420091634.128787-1-jgross@suse.com/
Juergen Gross (11):
x86/msr: Switch rdmsr_on_cpu() to return a 64-bit quantity
x86/msr: Switch all callers of rdmsrq_on_cpu() to use rdmsr_on_cpu()
x86/msr: Switch wrmsr_on_cpu() to use a 64-bit quantity
x86/msr: Switch all callers of wrmsrq_on_cpu() to use wrmsr_on_cpu()
x86/msr: Switch rdmsr_safe_on_cpu() to return a 64-bit quantity
x86/msr: Switch all callers of rdmsrq_safe_on_cpu() to use
rdmsr_safe_on_cpu()
x86/msr: Switch wrmsr_safe_on_cpu() to use a 64-bit quantity
x86/msr: Switch all callers of wrmsrq_safe_on_cpu() to use
wrmsr_safe_on_cpu()
x86/msr: Add macros for preparing to switch rdmsr/wrmsr interfaces
x86/events: Switch core parts to use 64-bit rdmsr/wrmsr() variants
x86/cpu/mce: Switch code to use 64-bit rdmsr/wrmsr() variants
arch/x86/events/core.c | 42 ++++----
arch/x86/events/intel/ds.c | 11 +-
arch/x86/events/intel/pt.c | 2 +-
arch/x86/events/intel/uncore_discovery.c | 2 +-
arch/x86/events/intel/uncore_snbep.c | 2 +-
arch/x86/events/msr.c | 2 +-
arch/x86/events/perf_event.h | 26 ++---
arch/x86/events/probe.c | 2 +-
arch/x86/events/rapl.c | 8 +-
arch/x86/include/asm/msr.h | 90 +++++++++-------
arch/x86/include/asm/paravirt.h | 6 +-
arch/x86/kernel/acpi/cppc.c | 8 +-
arch/x86/kernel/cpu/intel_epb.c | 8 +-
arch/x86/kernel/cpu/mce/amd.c | 101 +++++++++---------
arch/x86/kernel/cpu/mce/core.c | 18 ++--
arch/x86/kernel/cpu/mce/inject.c | 40 +++----
arch/x86/kernel/cpu/mce/intel.c | 32 +++---
arch/x86/kernel/cpu/mce/p5.c | 16 +--
arch/x86/kernel/cpu/mce/winchip.c | 10 +-
arch/x86/kernel/cpu/microcode/intel.c | 2 +-
arch/x86/kernel/msr.c | 8 +-
arch/x86/lib/msr-smp.c | 79 ++------------
drivers/cpufreq/acpi-cpufreq.c | 4 +-
drivers/cpufreq/amd-pstate-ut.c | 2 +-
drivers/cpufreq/amd-pstate.c | 21 ++--
drivers/cpufreq/amd_freq_sensitivity.c | 4 +-
drivers/cpufreq/intel_pstate.c | 64 +++++------
drivers/cpufreq/p4-clockmod.c | 32 +++---
drivers/cpufreq/speedstep-centrino.c | 27 ++---
drivers/hwmon/coretemp.c | 44 ++++----
drivers/hwmon/via-cputemp.c | 16 +--
drivers/platform/x86/amd/hfi/hfi.c | 4 +-
.../intel/speed_select_if/isst_if_common.c | 13 ++-
.../intel/uncore-frequency/uncore-frequency.c | 12 +--
drivers/powercap/intel_rapl_msr.c | 2 +-
drivers/thermal/intel/intel_tcc.c | 43 ++++----
drivers/thermal/intel/x86_pkg_temp_thermal.c | 22 ++--
37 files changed, 387 insertions(+), 438 deletions(-)
--
2.53.0
^ permalink raw reply
* Re: [PATCH] vsock/virtio: fix memory leak in virtio_transport_recv_listen()
From: Stefano Garzarella @ 2026-04-28 8:57 UTC (permalink / raw)
To: Deepanshu Kartikey
Cc: mst, jasowang, xuanzhuo, eperezma, stefanha, davem, edumazet,
kuba, pabeni, horms, virtualization, kvm, netdev, linux-kernel,
syzbot+1b2c9c4a0f8708082678
In-Reply-To: <CADhLXY6APdmMQLSxN9b66pV5mp5sDwJRR7_BJAh13BaK-qoa=w@mail.gmail.com>
On Tue, Apr 28, 2026 at 01:54:36PM +0530, Deepanshu Kartikey wrote:
>On Mon, Apr 27, 2026 at 7:15 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>>
>> On Fri, Apr 24, 2026 at 08:33:10PM +0530, Deepanshu Kartikey wrote:
>> >Two bugs exist in virtio_transport_recv_listen():
>>
>> Two bugs, two fixes, two patches usually.
>>
>> >
>> >1. On the transport assignment error path, sk_acceptq_added() is called
>> > but sk_acceptq_removed() is never called when vsock_assign_transport()
>> > fails or assigns a different transport than expected. This causes the
>> > parent listener's accept backlog counter to be permanently inflated,
>> > eventually causing sk_acceptq_is_full() to reject legitimate incoming
>> > connections.
>>
>> Wait, I can't see this issue. sk_acceptq_added() is called after the
>> vsock_assign_transport(), so why we should call sk_acceptq_removed()
>> in the error path of vsock_assign_transport()?
>>
>> Maybe I'm missing something.
>>
>> >
>> >2. There is a race between __vsock_release() and vsock_enqueue_accept().
>> > __vsock_release() sets sk->sk_shutdown to SHUTDOWN_MASK and flushes
>> > the accept queue under the parent socket lock. However,
>> > virtio_transport_recv_listen() checks sk_shutdown and subsequently
>> > calls vsock_enqueue_accept() without holding the parent socket lock.
>>
>> Are you sure about this?
>>
>> virtio_transport_recv_listen() is called only by
>> virtio_transport_recv_pkt() after calling lock_sock(sk), so I'm really
>> confused.
>>
>> > This means a child socket can be enqueued after __vsock_release() has
>> > already flushed the queue, causing the child socket and its associated
>> > resources to leak
>> > permanently. The existing comment in the code hints at this race but
>> > the fix was never implemented.
>>
>> Are you referring to:
>> /* __vsock_release() might have already flushed accept_queue.
>> * Subsequent enqueues would lead to a memory leak.
>> */
>> if (sk->sk_shutdown == SHUTDOWN_MASK) {
>> virtio_transport_reset_no_sock(t, skb, sock_net(sk));
>> return -ESHUTDOWN;
>> }
>>
>> In this case I think we are saying that we are doing this check exactly
>> to avoid that issue.
>>
>> >
>> >Fix both issues: add sk_acceptq_removed() on the transport error path,
>>
>> Again, better to fix the 2 issues with 2 patches (same series is fine).
>>
>> >and re-check sk->sk_shutdown under the parent socket lock before calling
>> >vsock_enqueue_accept() to close the race window. The child socket lock
>> >is released before acquiring the parent socket lock to maintain correct
>> >lock ordering (parent before child).
>> >
>>
>> We are missing the Fixes tag, and I think we can target the `net` tree
>> with this patch (i.e. [PATCH net]), see:
>> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
>>
>> >Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
>> >Closes: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
>> >Tested-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
>> >Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
>> >---
>> > net/vmw_vsock/virtio_transport_common.c | 13 +++++++++++--
>> > 1 file changed, 11 insertions(+), 2 deletions(-)
>> >
>> >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> >index 416d533f493d..fad5fa4a4296 100644
>> >--- a/net/vmw_vsock/virtio_transport_common.c
>> >+++ b/net/vmw_vsock/virtio_transport_common.c
>> >@@ -1578,6 +1578,7 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
>> > */
>> > if (ret || vchild->transport != &t->transport) {
>> > release_sock(child);
>> >+ sk_acceptq_removed(sk);
>>
>> Ditto, are we sure about this?
>>
>> > virtio_transport_reset_no_sock(t, skb, sock_net(sk));
>> > sock_put(child);
>> > return ret;
>> >@@ -1588,11 +1589,19 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
>> > child->sk_write_space(child);
>> >
>> > vsock_insert_connected(vchild);
>> >+ release_sock(child);
>> >+ lock_sock(sk);
>>
>> IMO this is a deadlock with the lock_sock(sk) called by the caller.
>>
>> Also a comment would be helpful here to explain why we're doing this.
>>
>> >+ if (sk->sk_shutdown == SHUTDOWN_MASK) {
>> >+ release_sock(sk);
>> >+ sk_acceptq_removed(sk);
>> >+ virtio_transport_reset_no_sock(t, skb, sock_net(sk));
>> >+ sock_put(child);
>> >+ return -ESHUTDOWN;
>>
>> Since this is very similar to the error path of
>> vsock_assign_transport(), I think it would be better to start by
>> defining a common error path for the function and use labels to exit, so
>> we can avoid duplicating the code multiple times.
>>
>> >+ }
>> > vsock_enqueue_accept(sk, child);
>> >+ release_sock(sk);
>> > virtio_transport_send_response(vchild, skb);
>> >
>> >- release_sock(child);
>> >-
>>
>> TBH I'm really worried about this patch since both fixes are completely
>> wrong IMO.
>>
>> Thanks,
>> Stefano
>>
>> > sk->sk_data_ready(sk);
>> > return 0;
>> > }
>> >--
>> >2.43.0
>> >
>>
>
>
>Hi Stefano,
>
>Thank you for the detailed review!
>
>You are correct on both points. I apologize for the confusion — I was
>looking at an older version of the code where sk_acceptq_added() was
>called BEFORE vsock_assign_transport(), which made the
>sk_acceptq_removed() fix appear necessary. In the current kernel,
>sk_acceptq_added() is already moved to after vsock_assign_transport(),
>so that issue no longer exists.
>
>Regarding the lock_sock(sk) fix — you are also correct that
>virtio_transport_recv_pkt() already holds lock_sock(sk) before calling
>virtio_transport_recv_listen(), so our second fix would indeed cause a
>deadlock. I missed that completely.
>
>I am still investigating the root cause of the memory leak reported by
>syzbot. The backtrace points to the vsock loopback path
>(vsock_loopback_work), so I am looking there next. I will send a v2
>once I have a correct analysis and fix.
Okay, thanks for looking into that issue, feel free to chat here, or in
reply to the syzbot report if you have some new findings.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH] vsock/virtio: fix memory leak in virtio_transport_recv_listen()
From: Deepanshu Kartikey @ 2026-04-28 8:24 UTC (permalink / raw)
To: Stefano Garzarella
Cc: mst, jasowang, xuanzhuo, eperezma, stefanha, davem, edumazet,
kuba, pabeni, horms, virtualization, kvm, netdev, linux-kernel,
syzbot+1b2c9c4a0f8708082678
In-Reply-To: <ae9ilWoZI_-EMdWQ@sgarzare-redhat>
On Mon, Apr 27, 2026 at 7:15 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Fri, Apr 24, 2026 at 08:33:10PM +0530, Deepanshu Kartikey wrote:
> >Two bugs exist in virtio_transport_recv_listen():
>
> Two bugs, two fixes, two patches usually.
>
> >
> >1. On the transport assignment error path, sk_acceptq_added() is called
> > but sk_acceptq_removed() is never called when vsock_assign_transport()
> > fails or assigns a different transport than expected. This causes the
> > parent listener's accept backlog counter to be permanently inflated,
> > eventually causing sk_acceptq_is_full() to reject legitimate incoming
> > connections.
>
> Wait, I can't see this issue. sk_acceptq_added() is called after the
> vsock_assign_transport(), so why we should call sk_acceptq_removed()
> in the error path of vsock_assign_transport()?
>
> Maybe I'm missing something.
>
> >
> >2. There is a race between __vsock_release() and vsock_enqueue_accept().
> > __vsock_release() sets sk->sk_shutdown to SHUTDOWN_MASK and flushes
> > the accept queue under the parent socket lock. However,
> > virtio_transport_recv_listen() checks sk_shutdown and subsequently
> > calls vsock_enqueue_accept() without holding the parent socket lock.
>
> Are you sure about this?
>
> virtio_transport_recv_listen() is called only by
> virtio_transport_recv_pkt() after calling lock_sock(sk), so I'm really
> confused.
>
> > This means a child socket can be enqueued after __vsock_release() has
> > already flushed the queue, causing the child socket and its associated
> > resources to leak
> > permanently. The existing comment in the code hints at this race but
> > the fix was never implemented.
>
> Are you referring to:
> /* __vsock_release() might have already flushed accept_queue.
> * Subsequent enqueues would lead to a memory leak.
> */
> if (sk->sk_shutdown == SHUTDOWN_MASK) {
> virtio_transport_reset_no_sock(t, skb, sock_net(sk));
> return -ESHUTDOWN;
> }
>
> In this case I think we are saying that we are doing this check exactly
> to avoid that issue.
>
> >
> >Fix both issues: add sk_acceptq_removed() on the transport error path,
>
> Again, better to fix the 2 issues with 2 patches (same series is fine).
>
> >and re-check sk->sk_shutdown under the parent socket lock before calling
> >vsock_enqueue_accept() to close the race window. The child socket lock
> >is released before acquiring the parent socket lock to maintain correct
> >lock ordering (parent before child).
> >
>
> We are missing the Fixes tag, and I think we can target the `net` tree
> with this patch (i.e. [PATCH net]), see:
> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
>
> >Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
> >Closes: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
> >Tested-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
> >Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> >---
> > net/vmw_vsock/virtio_transport_common.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> >index 416d533f493d..fad5fa4a4296 100644
> >--- a/net/vmw_vsock/virtio_transport_common.c
> >+++ b/net/vmw_vsock/virtio_transport_common.c
> >@@ -1578,6 +1578,7 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> > */
> > if (ret || vchild->transport != &t->transport) {
> > release_sock(child);
> >+ sk_acceptq_removed(sk);
>
> Ditto, are we sure about this?
>
> > virtio_transport_reset_no_sock(t, skb, sock_net(sk));
> > sock_put(child);
> > return ret;
> >@@ -1588,11 +1589,19 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> > child->sk_write_space(child);
> >
> > vsock_insert_connected(vchild);
> >+ release_sock(child);
> >+ lock_sock(sk);
>
> IMO this is a deadlock with the lock_sock(sk) called by the caller.
>
> Also a comment would be helpful here to explain why we're doing this.
>
> >+ if (sk->sk_shutdown == SHUTDOWN_MASK) {
> >+ release_sock(sk);
> >+ sk_acceptq_removed(sk);
> >+ virtio_transport_reset_no_sock(t, skb, sock_net(sk));
> >+ sock_put(child);
> >+ return -ESHUTDOWN;
>
> Since this is very similar to the error path of
> vsock_assign_transport(), I think it would be better to start by
> defining a common error path for the function and use labels to exit, so
> we can avoid duplicating the code multiple times.
>
> >+ }
> > vsock_enqueue_accept(sk, child);
> >+ release_sock(sk);
> > virtio_transport_send_response(vchild, skb);
> >
> >- release_sock(child);
> >-
>
> TBH I'm really worried about this patch since both fixes are completely
> wrong IMO.
>
> Thanks,
> Stefano
>
> > sk->sk_data_ready(sk);
> > return 0;
> > }
> >--
> >2.43.0
> >
>
Hi Stefano,
Thank you for the detailed review!
You are correct on both points. I apologize for the confusion — I was
looking at an older version of the code where sk_acceptq_added() was
called BEFORE vsock_assign_transport(), which made the
sk_acceptq_removed() fix appear necessary. In the current kernel,
sk_acceptq_added() is already moved to after vsock_assign_transport(),
so that issue no longer exists.
Regarding the lock_sock(sk) fix — you are also correct that
virtio_transport_recv_pkt() already holds lock_sock(sk) before calling
virtio_transport_recv_listen(), so our second fix would indeed cause a
deadlock. I missed that completely.
I am still investigating the root cause of the memory leak reported by
syzbot. The backtrace points to the vsock loopback path
(vsock_loopback_work), so I am looking there next. I will send a v2
once I have a correct analysis and fix.
Sorry again for the noise.
Thanks,
Deeranshu Kartikey
^ permalink raw reply
* Re: [PATCH 1/2] hv_sock: fix ARM64 support
From: Jakub Kicinski @ 2026-04-28 0:06 UTC (permalink / raw)
To: Hamza Mahfooz
Cc: linux-kernel, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Stefano Garzarella, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, Himadri Pandya,
Michael Kelley, linux-hyperv, virtualization, netdev,
Saurabh Sengar, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Deepak Rawat,
dri-devel, stable
In-Reply-To: <20260425181719.1538483-1-hamzamahfooz@linux.microsoft.com>
On Sat, 25 Apr 2026 11:17:18 -0700 Hamza Mahfooz wrote:
> VMBUS ring buffers must be page aligned. Therefore, the current value of
> 24K presents a challenge on ARM64 kernels (with 64K pages). So, use
> VMBUS_RING_SIZE() to ensure they are always aligned and large enough to
> hold all of the relevant data.
Please split the fixes into two independent postings.
They have to go via different trees AFAICT
^ permalink raw reply
* RE: [PATCH 2/2] drm/hyperv: use VMBUS_RING_SIZE()
From: Michael Kelley @ 2026-04-27 19:05 UTC (permalink / raw)
To: Dexuan Cui, Hamza Mahfooz, Saurabh Singh Sengar
Cc: linux-kernel@vger.kernel.org, KY Srinivasan, Haiyang Zhang,
Wei Liu, Long Li, Stefano Garzarella, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Himadri Pandya, linux-hyperv@vger.kernel.org,
virtualization@lists.linux.dev, netdev@vger.kernel.org,
Saurabh Sengar, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Deepak Rawat,
dri-devel@lists.freedesktop.org, stable@kernel.vger.org
In-Reply-To: <SA1PR21MB6921B5A2441DB3E9A1312AC0BF362@SA1PR21MB6921.namprd21.prod.outlook.com>
From: Dexuan Cui <DECUI@microsoft.com> Sent: Monday, April 27, 2026 11:42 AM
>
> > From: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
> > Sent: Monday, April 27, 2026 4:52 AM
> > To: Saurabh Singh Sengar <ssengar@microsoft.com>
> > ...
> > On Sun, Apr 26, 2026 at 05:00:24AM +0000, Saurabh Singh Sengar wrote:
> > > > Subject: [PATCH 2/2] drm/hyperv: use VMBUS_RING_SIZE()
> > > >
> > > > VMBUS ring buffers must be page aligned. So, use VMBUS_RING_SIZE() to
> > > > ensure they are always aligned and large enough to hold all of the relevant
> > > > data.
> > > >
> > > > Cc: stable@kernel.vger.org
> > > > Fixes: 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic
> > > > video device")
>
> IMO the Fixes tag is unnecessary because the existing VMBUS_RING_BUFSIZE
> is 256KB, which is already aligned to 4KB, 16KB and 64KB.
>
> VMBUS_RING_SIZE(256 * 1024) is still 256KB.
Not always. If PAGE_SIZE is 64KiB, VMBUS_RING_SIZE(256 * 1024) is
320KiB. If PAGE_SIZE is 16KiB or 4KiB, then VMBUS_RING_SIZE(256 * 1024)
is indeed 256 KiB. See the explanation in the comment for VMBUS_RING_SIZE.
Michael
>
> > > > Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
> > > > ---
> > > > drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> > > > b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> > > > index 051ecc526832..753d97bff76f 100644
> > > > --- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> > > > +++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> > > > @@ -10,7 +10,7 @@
> > > >
> > > > #include "hyperv_drm.h"
> > > >
> > > > -#define VMBUS_RING_BUFSIZE (256 * 1024)
> > > > +#define VMBUS_RING_BUFSIZE VMBUS_RING_SIZE(256 * 1024)
> > > > #define VMBUS_VSP_TIMEOUT (10 * HZ)
> > > >
> > > > #define SYNTHVID_VERSION(major, minor) ((minor) << 16 | (major))
> > > > --
> > > > 2.54.0
> > >
> > > Although this lgtm, but this may change the behaviour on ARM64 systems
> > with page size > 4K ?
>
> Actually the behavior won't change, because
> VMBUS_RING_SIZE(256 * 1024) is still 256KB.
>
> > > Have we tested it ?
> >
> > Yup, I tested it on an ARM64 windows machine with a 64K page size guest
> > kernel.
> >
> > >
> > > Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> >
> > Pushed to drm-misc.
^ permalink raw reply
* RE: [PATCH 1/2] hv_sock: fix ARM64 support
From: Dexuan Cui @ 2026-04-27 18:53 UTC (permalink / raw)
To: Hamza Mahfooz, linux-kernel@vger.kernel.org
Cc: KY Srinivasan, Haiyang Zhang, Wei Liu, Long Li,
Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Himadri Pandya, Michael Kelley,
linux-hyperv@vger.kernel.org, virtualization@lists.linux.dev,
netdev@vger.kernel.org, Saurabh Sengar, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Deepak Rawat, dri-devel@lists.freedesktop.org,
stable@kernel.vger.org
In-Reply-To: <20260425181719.1538483-1-hamzamahfooz@linux.microsoft.com>
> From: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
> Sent: Saturday, April 25, 2026 11:17 AM
> ...
> Cc: stable@kernel.vger.org
> Fixes: 77ffe33363c0 ("hv_sock: use HV_HYP_PAGE_SIZE for Hyper-V
> communication")
It looks like 77ffe33363c0 was not tested with
CONFIG_ARM64_64K_PAGES=y...
Thanks for the fix!
> Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
Tested-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Dexuan Cui <decui@microsoft.com>
^ permalink raw reply
* RE: [PATCH 2/2] drm/hyperv: use VMBUS_RING_SIZE()
From: Dexuan Cui @ 2026-04-27 18:42 UTC (permalink / raw)
To: Hamza Mahfooz, Saurabh Singh Sengar
Cc: linux-kernel@vger.kernel.org, KY Srinivasan, Haiyang Zhang,
Wei Liu, Long Li, Stefano Garzarella, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Himadri Pandya, Michael Kelley, linux-hyperv@vger.kernel.org,
virtualization@lists.linux.dev, netdev@vger.kernel.org,
Saurabh Sengar, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Deepak Rawat,
dri-devel@lists.freedesktop.org, stable@kernel.vger.org
In-Reply-To: <ae9NxmDBTkzPP3H6@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
> From: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
> Sent: Monday, April 27, 2026 4:52 AM
> To: Saurabh Singh Sengar <ssengar@microsoft.com>
> ...
> On Sun, Apr 26, 2026 at 05:00:24AM +0000, Saurabh Singh Sengar wrote:
> > > Subject: [PATCH 2/2] drm/hyperv: use VMBUS_RING_SIZE()
> > >
> > > VMBUS ring buffers must be page aligned. So, use VMBUS_RING_SIZE() to
> > > ensure they are always aligned and large enough to hold all of the relevant
> > > data.
> > >
> > > Cc: stable@kernel.vger.org
> > > Fixes: 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic
> > > video device")
IMO the Fixes tag is unnecessary because the existing VMBUS_RING_BUFSIZE
is 256KB, which is already aligned to 4KB, 16KB and 64KB.
VMBUS_RING_SIZE(256 * 1024) is still 256KB.
> > > Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
> > > ---
> > > drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> > > b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> > > index 051ecc526832..753d97bff76f 100644
> > > --- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> > > +++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> > > @@ -10,7 +10,7 @@
> > >
> > > #include "hyperv_drm.h"
> > >
> > > -#define VMBUS_RING_BUFSIZE (256 * 1024)
> > > +#define VMBUS_RING_BUFSIZE VMBUS_RING_SIZE(256 * 1024)
> > > #define VMBUS_VSP_TIMEOUT (10 * HZ)
> > >
> > > #define SYNTHVID_VERSION(major, minor) ((minor) << 16 | (major))
> > > --
> > > 2.54.0
> >
> > Although this lgtm, but this may change the behaviour on ARM64 systems
> with page size > 4K ?
Actually the behavior won't change, because
VMBUS_RING_SIZE(256 * 1024) is still 256KB.
> > Have we tested it ?
>
> Yup, I tested it on an ARM64 windows machine with a 64K page size guest
> kernel.
>
> >
> > Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
>
> Pushed to drm-misc.
^ permalink raw reply
* [RFC PATCH v6 4/4] virtio-mmio: wire up noirq system sleep PM callbacks
From: Sungho Bae @ 2026-04-27 17:30 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, virtualization, linux-kernel, Sungho Bae
In-Reply-To: <20260427173045.28652-1-baver.bae@gmail.com>
From: Sungho Bae <baver.bae@lge.com>
Add noirq system-sleep PM support to the virtio-mmio transport.
This change wires noirq freeze/restore callbacks into virtio-mmio and
hooks queue reset/reactivation into the transport config ops so virtqueues
can be reinitialized and reused across suspend/resume.
For legacy (v1) devices, keep GUEST_PAGE_SIZE programming aligned with the
noirq restore path while avoiding duplicate programming in normal restore.
This enables virtio-mmio based devices to participate safely in the noirq
PM phase, which is required for early-restore users.
Signed-off-by: Sungho Bae <baver.bae@lge.com>
---
drivers/virtio/virtio_mmio.c | 136 ++++++++++++++++++++++++-----------
1 file changed, 96 insertions(+), 40 deletions(-)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 595c2274fbb5..71d13cb57c43 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -336,6 +336,77 @@ static void vm_del_vqs(struct virtio_device *vdev)
free_irq(platform_get_irq(vm_dev->pdev, 0), vm_dev);
}
+static int vm_active_vq(struct virtio_device *vdev, struct virtqueue *vq)
+{
+ struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
+ int q_num = virtqueue_get_vring_size(vq);
+
+ writel(q_num, vm_dev->base + VIRTIO_MMIO_QUEUE_NUM);
+ if (vm_dev->version == 1) {
+ u64 q_pfn = virtqueue_get_desc_addr(vq) >> PAGE_SHIFT;
+
+ /*
+ * virtio-mmio v1 uses a 32bit QUEUE PFN. If we have something
+ * that doesn't fit in 32bit, fail the setup rather than
+ * pretending to be successful.
+ */
+ if (q_pfn >> 32) {
+ dev_err(&vdev->dev,
+ "platform bug: legacy virtio-mmio must not be used with RAM above 0x%llxGB\n",
+ 0x1ULL << (32 + PAGE_SHIFT - 30));
+ return -E2BIG;
+ }
+
+ writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_QUEUE_ALIGN);
+ writel(q_pfn, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
+ } else {
+ u64 addr;
+
+ addr = virtqueue_get_desc_addr(vq);
+ writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_LOW);
+ writel((u32)(addr >> 32),
+ vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_HIGH);
+
+ addr = virtqueue_get_avail_addr(vq);
+ writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_LOW);
+ writel((u32)(addr >> 32),
+ vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_HIGH);
+
+ addr = virtqueue_get_used_addr(vq);
+ writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_USED_LOW);
+ writel((u32)(addr >> 32),
+ vm_dev->base + VIRTIO_MMIO_QUEUE_USED_HIGH);
+
+ writel(1, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
+ }
+
+ return 0;
+}
+
+static int vm_reset_vqs(struct virtio_device *vdev)
+{
+ struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
+ struct virtqueue *vq;
+ int err;
+
+ virtio_device_for_each_vq(vdev, vq) {
+ /* Re-initialize vring state */
+ err = virtqueue_reinit_vring(vq);
+ if (err < 0)
+ return err;
+
+ /* Select the queue we're interested in */
+ writel(vq->index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
+
+ /* Activate the queue */
+ err = vm_active_vq(vdev, vq);
+ if (err < 0)
+ return err;
+ }
+
+ return 0;
+}
+
static void vm_synchronize_cbs(struct virtio_device *vdev)
{
struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
@@ -388,45 +459,9 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in
vq->num_max = num;
/* Activate the queue */
- writel(virtqueue_get_vring_size(vq), vm_dev->base + VIRTIO_MMIO_QUEUE_NUM);
- if (vm_dev->version == 1) {
- u64 q_pfn = virtqueue_get_desc_addr(vq) >> PAGE_SHIFT;
-
- /*
- * virtio-mmio v1 uses a 32bit QUEUE PFN. If we have something
- * that doesn't fit in 32bit, fail the setup rather than
- * pretending to be successful.
- */
- if (q_pfn >> 32) {
- dev_err(&vdev->dev,
- "platform bug: legacy virtio-mmio must not be used with RAM above 0x%llxGB\n",
- 0x1ULL << (32 + PAGE_SHIFT - 30));
- err = -E2BIG;
- goto error_bad_pfn;
- }
-
- writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_QUEUE_ALIGN);
- writel(q_pfn, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
- } else {
- u64 addr;
-
- addr = virtqueue_get_desc_addr(vq);
- writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_LOW);
- writel((u32)(addr >> 32),
- vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_HIGH);
-
- addr = virtqueue_get_avail_addr(vq);
- writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_LOW);
- writel((u32)(addr >> 32),
- vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_HIGH);
-
- addr = virtqueue_get_used_addr(vq);
- writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_USED_LOW);
- writel((u32)(addr >> 32),
- vm_dev->base + VIRTIO_MMIO_QUEUE_USED_HIGH);
-
- writel(1, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
- }
+ err = vm_active_vq(vdev, vq);
+ if (err < 0)
+ goto error_bad_pfn;
return vq;
@@ -528,11 +563,13 @@ static const struct virtio_config_ops virtio_mmio_config_ops = {
.reset = vm_reset,
.find_vqs = vm_find_vqs,
.del_vqs = vm_del_vqs,
+ .reset_vqs = vm_reset_vqs,
.get_features = vm_get_features,
.finalize_features = vm_finalize_features,
.bus_name = vm_bus_name,
.get_shm_region = vm_get_shm_region,
.synchronize_cbs = vm_synchronize_cbs,
+ .noirq_safe = true,
};
#ifdef CONFIG_PM_SLEEP
@@ -547,14 +584,33 @@ static int virtio_mmio_restore(struct device *dev)
{
struct virtio_mmio_device *vm_dev = dev_get_drvdata(dev);
- if (vm_dev->version == 1)
+ if (vm_dev->version == 1 && !vm_dev->vdev.noirq_restore_done)
writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
return virtio_device_restore(&vm_dev->vdev);
}
+static int virtio_mmio_freeze_noirq(struct device *dev)
+{
+ struct virtio_mmio_device *vm_dev = dev_get_drvdata(dev);
+
+ return virtio_device_freeze_noirq(&vm_dev->vdev);
+}
+
+static int virtio_mmio_restore_noirq(struct device *dev)
+{
+ struct virtio_mmio_device *vm_dev = dev_get_drvdata(dev);
+
+ if (vm_dev->version == 1)
+ writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
+
+ return virtio_device_restore_noirq(&vm_dev->vdev);
+}
+
static const struct dev_pm_ops virtio_mmio_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(virtio_mmio_freeze, virtio_mmio_restore)
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(virtio_mmio_freeze_noirq,
+ virtio_mmio_restore_noirq)
};
#endif
--
2.43.0
^ permalink raw reply related
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