* Re: [PATCH v2] mm: page_reporting: allow driver to set batch capacity
From: Gupta, Pankaj @ 2026-06-15 9:18 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Miaohe Lin, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Perez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi,
Alexander Duyck
In-Reply-To: <cb43adc61d2ed3069b2fe428f3e051dbdc4cc28d.1781097156.git.mst@redhat.com>
> At the moment, if a virtio balloon device has a page reporting vq but
> its size is < PAGE_REPORTING_CAPACITY (32), the balloon driver fails
> probe.
>
> But, there's no way for host to know this value, so it can easily
> create a smaller vq and suddenly adding the reporting capability
> to the device makes all of the driver fail. Not pretty.
>
> Add a capacity field to page_reporting_dev_info so drivers can
> control the maximum number of pages per report batch.
>
> In virtio-balloon, set the capacity to the reporting virtqueue size,
> letting page_reporting adapt to whatever the device provides.
>
> Capacity need not be a power of two. Code previously called out
> division by PAGE_REPORTING_CAPACITY as cheap since it was a power
> of 2, but no performance difference was observed with non-power-of-2
> values.
>
> If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> to PAGE_REPORTING_CAPACITY. The 0 check and the clamping is done in
> page_reporting_register(), before the reporting work is scheduled,
> so we never get division by 0.
>
> Fixes: b0c504f15471 ("virtio-balloon: add support for providing free page reports to host")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---
> Changes v1->v2:
> - Document capacity=0 as default in commit log
> - Document that capacity need not be a power of two
> - Drop unnecessary comment about integer division cost
> - Update comment on capacity field: "0 (default) means PAGE_REPORTING_CAPACITY"
>
> drivers/virtio/virtio_balloon.c | 5 +----
> include/linux/page_reporting.h | 3 +++
> mm/page_reporting.c | 24 ++++++++++++------------
> 3 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index f6c2dff33f8a..6a1a610c2cb1 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -1017,10 +1017,6 @@ static int virtballoon_probe(struct virtio_device *vdev)
> unsigned int capacity;
>
> capacity = virtqueue_get_vring_size(vb->reporting_vq);
> - if (capacity < PAGE_REPORTING_CAPACITY) {
> - err = -ENOSPC;
> - goto out_unregister_oom;
> - }
>
> vb->pr_dev_info.order = PAGE_REPORTING_ORDER_UNSPECIFIED;
>
> @@ -1041,6 +1037,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
> vb->pr_dev_info.order = 5;
> #endif
>
> + vb->pr_dev_info.capacity = capacity;
> err = page_reporting_register(&vb->pr_dev_info);
> if (err)
> goto out_unregister_oom;
> diff --git a/include/linux/page_reporting.h b/include/linux/page_reporting.h
> index 9d4ca5c218a0..048578118a4b 100644
> --- a/include/linux/page_reporting.h
> +++ b/include/linux/page_reporting.h
> @@ -22,6 +22,9 @@ struct page_reporting_dev_info {
>
> /* Minimal order of page reporting */
> unsigned int order;
> +
> + /* Max pages per report batch; 0 (default) means PAGE_REPORTING_CAPACITY */
> + unsigned int capacity;
> };
>
> /* Tear-down and bring-up for page reporting devices */
> diff --git a/mm/page_reporting.c b/mm/page_reporting.c
> index 7418f2e500bb..942e84b6908a 100644
> --- a/mm/page_reporting.c
> +++ b/mm/page_reporting.c
> @@ -173,11 +173,8 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
> * any pages that may have already been present from the previous
> * list processed. This should result in us reporting all pages on
> * an idle system in about 30 seconds.
> - *
> - * The division here should be cheap since PAGE_REPORTING_CAPACITY
> - * should always be a power of 2.
> */
> - budget = DIV_ROUND_UP(area->nr_free, PAGE_REPORTING_CAPACITY * 16);
> + budget = DIV_ROUND_UP(area->nr_free, prdev->capacity * 16);
>
> /* loop through free list adding unreported pages to sg list */
> list_for_each_entry_safe(page, next, list, lru) {
> @@ -222,10 +219,10 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
> spin_unlock_irq(&zone->lock);
>
> /* begin processing pages in local list */
> - err = prdev->report(prdev, sgl, PAGE_REPORTING_CAPACITY);
> + err = prdev->report(prdev, sgl, prdev->capacity);
>
> /* reset offset since the full list was reported */
> - *offset = PAGE_REPORTING_CAPACITY;
> + *offset = prdev->capacity;
>
> /* update budget to reflect call to report function */
> budget--;
> @@ -234,7 +231,7 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
> spin_lock_irq(&zone->lock);
>
> /* flush reported pages from the sg list */
> - page_reporting_drain(prdev, sgl, PAGE_REPORTING_CAPACITY, !err);
> + page_reporting_drain(prdev, sgl, prdev->capacity, !err);
>
> /*
> * Reset next to first entry, the old next isn't valid
> @@ -260,13 +257,13 @@ static int
> page_reporting_process_zone(struct page_reporting_dev_info *prdev,
> struct scatterlist *sgl, struct zone *zone)
> {
> - unsigned int order, mt, leftover, offset = PAGE_REPORTING_CAPACITY;
> + unsigned int order, mt, leftover, offset = prdev->capacity;
> unsigned long watermark;
> int err = 0;
>
> /* Generate minimum watermark to be able to guarantee progress */
> watermark = low_wmark_pages(zone) +
> - (PAGE_REPORTING_CAPACITY << page_reporting_order);
> + (prdev->capacity << page_reporting_order);
>
> /*
> * Cancel request if insufficient free memory or if we failed
> @@ -290,7 +287,7 @@ page_reporting_process_zone(struct page_reporting_dev_info *prdev,
> }
>
> /* report the leftover pages before going idle */
> - leftover = PAGE_REPORTING_CAPACITY - offset;
> + leftover = prdev->capacity - offset;
> if (leftover) {
> sgl = &sgl[offset];
> err = prdev->report(prdev, sgl, leftover);
> @@ -322,11 +319,11 @@ static void page_reporting_process(struct work_struct *work)
> atomic_set(&prdev->state, state);
>
> /* allocate scatterlist to store pages being reported on */
> - sgl = kmalloc_objs(*sgl, PAGE_REPORTING_CAPACITY);
> + sgl = kmalloc_objs(*sgl, prdev->capacity);
> if (!sgl)
> goto err_out;
>
> - sg_init_table(sgl, PAGE_REPORTING_CAPACITY);
> + sg_init_table(sgl, prdev->capacity);
>
> for_each_zone(zone) {
> err = page_reporting_process_zone(prdev, sgl, zone);
> @@ -377,6 +374,9 @@ int page_reporting_register(struct page_reporting_dev_info *prdev)
> page_reporting_order = pageblock_order;
> }
>
> + if (!prdev->capacity || prdev->capacity > PAGE_REPORTING_CAPACITY)
> + prdev->capacity = PAGE_REPORTING_CAPACITY;
> +
> /* initialize state and work structures */
> atomic_set(&prdev->state, PAGE_REPORTING_IDLE);
> INIT_DELAYED_WORK(&prdev->work, &page_reporting_process);
With the comment change pointed by David,
Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com>
^ permalink raw reply
* [PATCH] virtio_net: disable cb when napi_schedule_prep fails during busy-poll
From: Longjun Tang @ 2026-06-15 9:45 UTC (permalink / raw)
To: mst, jasowang
Cc: edumazet, kuba, xuanzhuo, virtualization, lange_tang, tanglongjun
From: Longjun Tang <tanglongjun@kylinos.cn>
When busy-poll is active, napi_schedule_prep() returns false in
skb_recv_done(), so virtqueue_disable_cb() is skipped. The device
may keep firing irqs until the next poll round reaches
virtqueue_napi_complete(). If cb is enabled under busy-poll case,
it will lead to a large number of spurious interrupts. Explicitly
disable callbacks in this case to prevent spurious interrupts.
Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
---
drivers/net/virtio_net.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f4adcfee7a80..6d675fddc59b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -728,6 +728,8 @@ static void virtqueue_napi_schedule(struct napi_struct *napi,
if (napi_schedule_prep(napi)) {
virtqueue_disable_cb(vq);
__napi_schedule(napi);
+ } else if (test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state)) {
+ virtqueue_disable_cb(vq);
}
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] virtio_net: disable cb when napi_schedule_prep fails during busy-poll
From: Xuan Zhuo @ 2026-06-15 10:01 UTC (permalink / raw)
To: Longjun Tang
Cc: edumazet, kuba, virtualization, lange_tang, tanglongjun, mst,
jasowang
In-Reply-To: <20260615094550.106391-1-lange_tang@163.com>
On Mon, 15 Jun 2026 17:45:50 +0800, Longjun Tang <lange_tang@163.com> wrote:
> From: Longjun Tang <tanglongjun@kylinos.cn>
>
> When busy-poll is active, napi_schedule_prep() returns false in
> skb_recv_done(), so virtqueue_disable_cb() is skipped. The device
> may keep firing irqs until the next poll round reaches
> virtqueue_napi_complete(). If cb is enabled under busy-poll case,
> it will lead to a large number of spurious interrupts. Explicitly
> disable callbacks in this case to prevent spurious interrupts.
>
> Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
> ---
> drivers/net/virtio_net.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f4adcfee7a80..6d675fddc59b 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -728,6 +728,8 @@ static void virtqueue_napi_schedule(struct napi_struct *napi,
> if (napi_schedule_prep(napi)) {
> virtqueue_disable_cb(vq);
> __napi_schedule(napi);
> + } else if (test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state)) {
> + virtqueue_disable_cb(vq);
I see, but we should avoid checking NAPI_STATE_IN_BUSY_POLL directly in the
drivers. The NIC driver should remain agnostic to busy polling. I think we need
a better way, maybe we should rewrite virtqueue_napi_schedule instead.
Thanks.
> }
> }
>
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: David Hildenbrand (Arm) @ 2026-06-15 10:54 UTC (permalink / raw)
To: Miaohe Lin, Michael S. Tsirkin
Cc: Zi Yan, Andrew Morton, linux-kernel, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Hugh Dickins, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi
In-Reply-To: <984d9775-e17c-0231-b021-126b13a9aa42@huawei.com>
On 6/15/26 05:29, Miaohe Lin wrote:
> On 2026/6/11 21:20, David Hildenbrand (Arm) wrote:
>> On 6/11/26 09:36, Miaohe Lin wrote:
>>>
>>> Agree, it's not worth to do so.
>>>
>>>
>>> Since memory_failure might be the only place, this change would be unacceptable.
>>> We should come up with a better solution. Maybe we can try repeating SetPageHWPoison
>>> and ClearPageHWPoison at a first attempt though it looks somewhat weird to me and makes
>>> code more complicated.
>>
>> And I am fairly sure we could still have some remaining races ... it's shaky.
>
> I have to agree it's shaky.
Right, just let writing task reschedule after reading the flags,
but before writing the flags.
> Any suggestion for next step?
We have various code that assumes that no concurrent writes are
possible, and consequently, we use no atomics.
__free_pages_prepare() is just one user.
Then we have __folio_set_locked(), __folio_clear_active()
and __folio_clear_unevictable().
But also __folio_mark_uptodate(), which is called rather frequently.
page_cpupid_reset_last() is also a thing, but it mostly falls
under __free_pages_prepare() handling.
... and __split_folio_to_order() also messes with flags directly without atomics.
Many of these are only possible for frozen pages (refcount == 0). I think
only __folio_set_locked() and __folio_mark_uptodate() are called on
non-frozen pages, when there is the expectation that nobody will concurrently
use atomics that would be bad (e.g., don't trylock if not an lru page).
We don't want to use atomics at these places just to please memory failure code.
Would it be sufficient to know in memory-failure code that concurrent
handling succeeded?
Assume that we enlighten all non-atomics to grab the rcu read lock, such as
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 7223f6f4e2b4..3c3852b60bbd 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -803,10 +803,30 @@ static inline bool PageUptodate(const struct page *page)
return folio_test_uptodate(page_folio(page));
}
+#ifdef CONFIG_MEMORY_FAILURE
+static inline void page_flags_modify_nonatomic_begin(void)
+{
+ rcu_read_lock();
+}
+static inline void page_flags_modify_nonatomic_end(void)
+{
+ rcu_read_unlock();
+}
+#else
+static inline void page_flags_modify_nonatomic_begin(void)
+{
+}
+static inline void page_flags_modify_nonatomic_end(void)
+{
+}
+#endif
+
static __always_inline void __folio_mark_uptodate(struct folio *folio)
{
smp_wmb();
+ page_flags_modify_nonatomic_begin();
__set_bit(PG_uptodate, folio_flags(folio, 0));
+ page_flags_modify_nonatomic_end();
}
And then we have some retry logic such as:
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 51508a55c405..1123c40aaf43 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -162,6 +162,62 @@ static struct rb_root_cached pfn_space_itree = RB_ROOT_CACHED;
static DEFINE_MUTEX(pfn_space_lock);
+static bool page_test_set_hwpoison(struct page *page)
+{
+ lockdep_assert_held(&mf_mutex);
+
+ while (true) {
+ /* Already set -> not our problem. */
+ if (TestSetPageHWPoison(page))
+ return true;
+ /* Make sure concurrent non-atomic writers completed. */
+ synchronize_rcu();
+ /* Setting the flag was sticky. */
+ if (PageHWPoison(page))
+ return false;
+ }
+}
+
+static bool page_test_clear_hwpoison(struct page *page)
+{
+ lockdep_assert_held(&mf_mutex);
+
+ while (true) {
+ /* Already clear -> not our problem. */
+ if (!TestClearPageHWPoison(page))
+ return false;
+ /* Make sure concurrent non-atomic writers completed. */
+ synchronize_rcu();
+ /* Clearing the flag was sticky. */
+ if (!PageHWPoison(page))
+ return true;
+ }
+}
+
+static void page_set_hwpoison(struct page *page)
+{
+ lockdep_assert_held(&mf_mutex);
+
+ while (!PageHWPoison(page)) {
+ SetPageHWPoison(page);
+
+ /* Make sure concurrent non-atomic writers completed. */
+ synchronize_rcu();
+ }
+}
+
+static void page_clear_hwpoison(struct page *page)
+{
+ lockdep_assert_held(&mf_mutex);
+
+ while (PageHWPoison(page)) {
+ ClearPageHWPoison(page);
+
+ /* Make sure concurrent non-atomic writers completed. */
+ synchronize_rcu();
+ }
+}
+
/*
* Return values:
* 1: the page is dissolved (if needed) and taken off from buddy,
@@ -199,7 +255,7 @@ static bool page_handle_poison(struct page *page, bool hugepage_or_freepage, boo
return false;
}
- SetPageHWPoison(page);
+ page_set_hwpoison(page);
if (release)
put_page(page);
page_ref_inc(page);
@@ -1744,7 +1800,7 @@ static int mf_generic_kill_procs(unsigned long long pfn, int flags,
* Use this flag as an indication that the dax page has been
* remapped UC to prevent speculative consumption of poison.
*/
- SetPageHWPoison(&folio->page);
+ page_set_hwpoison(&folio->page);
/*
* Unlike System-RAM there is no possibility to swap in a
@@ -1789,7 +1845,7 @@ int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
goto unlock;
if (!pre_remove)
- SetPageHWPoison(page);
+ page_set_hwpoison(page);
/*
* The pre_remove case is revoking access, the memory is still
@@ -1866,7 +1922,7 @@ static unsigned long __folio_free_raw_hwp(struct folio *folio, bool move_flag)
head = llist_del_all(raw_hwp_list_head(folio));
llist_for_each_entry_safe(p, next, head, node) {
if (move_flag)
- SetPageHWPoison(p->page);
+ page_set_hwpoison(p->page);
else
num_poisoned_pages_sub(page_to_pfn(p->page), 1);
kfree(p);
@@ -2380,7 +2436,7 @@ int memory_failure(unsigned long pfn, int flags)
if (res != -ENOENT)
goto unlock_mutex;
- if (TestSetPageHWPoison(p)) {
+ if (page_test_set_hwpoison(p)) {
res = -EHWPOISON;
if (flags & MF_ACTION_REQUIRED)
res = kill_accessing_process(current, pfn, flags);
@@ -2410,7 +2466,7 @@ int memory_failure(unsigned long pfn, int flags)
} else {
/* We lost the race, try again */
if (retry) {
- ClearPageHWPoison(p);
+ page_clear_hwpoison(p);
retry = false;
goto try_again;
}
@@ -2431,7 +2487,7 @@ int memory_failure(unsigned long pfn, int flags)
/* filter pages that are protected from hwpoison test by users */
folio_lock(folio);
if (hwpoison_filter(p)) {
- ClearPageHWPoison(p);
+ page_clear_hwpoison(p);
folio_unlock(folio);
folio_put(folio);
res = -EOPNOTSUPP;
@@ -2751,7 +2807,7 @@ int unpoison_memory(unsigned long pfn)
}
folio_put(folio);
- if (TestClearPageHWPoison(p)) {
+ if (page_test_clear_hwpoison(p)) {
folio_put(folio);
ret = 0;
}
Maybe that would work. There would still be issues to solve
(a) We don't hold the mf_mutex on all call paths, but we really need it so a
page_test_set_hwpoison() cannot race in weird ways with the other primitives I think.
(b) There are some leftover SetPageHWPoison etc. instances. The ones in
arch/x86/kernel/cpu/mce/core.c likely cannot grab the mutex, but maybe they are
corner cases either way and we can document the situation.
Further, while I assume the synchronize_rcu() on the MCE path should be fine
(who cares about performance there?), I don't know if the added RCU read lock
on some paths could be noticable.
So one idea worth discussing, but I am sure there are more problems.
--
Cheers,
David
^ permalink raw reply related
* Re: vhost: fix vhost_get_avail_idx for a non empty ring
From: Christian Borntraeger @ 2026-06-15 14:24 UTC (permalink / raw)
To: mst
Cc: eperezma, jasowang, kvm, linux-kernel, netdev, sgarzare, shuangyu,
stefanha, virtualization, Christian Borntraeger
In-Reply-To: <559b04ae6ce52973c535dc47e461638b7f4c3d63.1772441455.git.mst@redhat.com>
Late feedback, but this patch massively improves our uperf latency/bandwidth
and cpu consumption significantly for s390. Improvements are all over
the place, streaming, transactional (100 byte/2000 byte). Nice fix.
Christian
^ permalink raw reply
* Re: [PATCH v1 0/2] virtio: PCI ERS permanent failure teardown for virtio-blk
From: Stefan Hajnoczi @ 2026-06-15 14:52 UTC (permalink / raw)
To: Xixin Liu
Cc: linux-block, virtualization, mst, jasowang, xuanzhuo, eperezma,
pbonzini, axboe, linux-kernel, Parav Pandit
In-Reply-To: <cover.virtio-blk-ers-v1.1780449274.git.liuxixin@kylinos.cn>
[-- Attachment #1: Type: text/plain, Size: 1472 bytes --]
On Mon, Jun 15, 2026 at 10:00:00AM +0800, Xixin Liu wrote:
> Hi,
>
> This series adds proper PCI AER error recovery handling for virtio-pci and
> completes virtio-blk teardown when ERS reports pci_channel_io_perm_failure.
CCing Parav because he previously looked at surprise removal:
https://lore.kernel.org/virtualization/20250822091706.21170-1-parav@nvidia.com/
>
> virtio-pci only registered reset_prepare/reset_done. The recovery core
> treats devices without error_detected as NO_AER_DRIVER and does not
> deliver perm_failure to the driver after a failed recovery. When bus
> reset fails (reproduced on QEMU with DLLLA not set within 100 ms after
> secondary bus reset), virtio-blk disks stay live even though virtqueues
> may already have been torn down during the frozen phase.
>
> Patch 1 registers error_detected (frozen quiesce + perm_failure notify).
> Patch 2 calls the virtio driver shutdown hook from virtio-pci on
> perm_failure, implements virtio-blk shutdown with blk_mark_disk_dead(),
> and fail-fast guards in virtio_queue_rq.
>
> Thanks,
> Xixin Liu
>
> ---
>
> Xixin Liu (2):
> virtio-pci: add error_detected for PCI AER recovery
> virtio-blk: mark disk dead on ERS permanent failure
>
> drivers/block/virtio_blk.c | 39 +++++++++++++++++++++++++++++++
> drivers/virtio/virtio_pci_common.c | 47 ++++++++++++++++++++++++++++++++++
> 2 files changed, 85 insertions(+)
>
> --
> 2.43.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v1] s390/virtio_ccw: Also suppress -EINVAL on device detach
From: Cornelia Huck @ 2026-06-15 14:58 UTC (permalink / raw)
To: Halil Pasic, William Bezenah
Cc: linux-s390, farman, hca, gor, agordeev, borntraeger, svens,
mjrosato, vneethv, oberpar, virtualization, kvm, linux-kernel,
Halil Pasic
In-Reply-To: <20260615002309.052e0614.pasic@linux.ibm.com>
On Mon, Jun 15 2026, Halil Pasic <pasic@linux.ibm.com> wrote:
> On Fri, 12 Jun 2026 17:54:07 +0200
> William Bezenah <wbezenah@linux.ibm.com> wrote:
>
>> Since commit 8c58a229688c ("s390/cio: Do not unregister the
>> subchannel based on DNV"), subchannel behavior following a device
>> detach has been updated and results in -EINVAL being propagated
>> rather than -ENODEV, originating from ccw_device_start_timeout_key()
>> in cio/device_ops. In the end, the virtio driver has no ability to
>> react to the difference between device and subchannel states here,
>> and during detach, both -ENODEV and -EINVAL indicate the device
>> cannot be used and should not be treated as errors requiring
>> attention. Update error handling in virtio_ccw_del_vq() and
>> virtio_ccw_drop_indicator() to suppress -EINVAL in addition to
>> -ENODEV.
>
> Hi William!
>
> Are you saying that ccw_device_start() started returning -EINVAL
> since 8c58a229688c ("s390/cio: Do not unregister the subchannel based on
> DNV")? Or did I somehow read the paragraph wrong?
>
> The funcition ccw_device_start is documented to return:
> * Returns:
> * %0, if the operation was successful;
> * -%EBUSY, if the device is busy, or status pending;
> * -%EACCES, if no path specified in @lpm is operational;
> * -%ENODEV, if the device is not operational.
> and the commit message does not say a thing about introducing -EINVAL to
> the mix.
The function may return -EINVAL for non-enabled subchannels
(i.e. pmcw.ena == 0), maybe we get an all-zeroes schib with dnv == 0?
I'd expect it not to be enabled in that case anyway.
^ permalink raw reply
* Re: [PATCH splitout] virtio_balloon: disable indirect descriptors
From: David Hildenbrand (Arm) @ 2026-06-15 16:11 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Miaohe Lin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Muchun Song, Oscar Salvador, Andrew Morton, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi,
Alexander Duyck
In-Reply-To: <7160278ae1fc4f06dec966867d307033989d029c.1781022765.git.mst@redhat.com>
On 6/9/26 18:33, Michael S. Tsirkin wrote:
> The page reporting callback submits an sg list to the reporting
> virtqueue. With VIRTIO_RING_F_INDIRECT_DESC negotiated and
> total_sg > 1 (which it typically is), virtqueue_add reports it to the
> host by allocating an indirect descriptor via kmalloc(GFP_KERNEL).
>
> This is not pretty: the reporting worker isolates potentially hundreds
> of MB of free pages from the buddy allocator (reported pages are at
> least pageblock_order, and the sg can contain up to
> PAGE_REPORTING_CAPACITY entries of varying orders). As the result, at
> least in theory, the kmalloc might trigger OOM when we have in fact a
> ton of free memory.
Very theoretical, given that we isolate large pageblocks and the kmalloc would
just need likely a single page. But yeah, avodiing to allocate memory where
possible on these paths makes sense I guess.
>
> Clear VIRTIO_RING_F_INDIRECT_DESC, to avoid using indirect descriptors.
>
> Fixes: b0c504f15471 ("virtio-balloon: add support for providing free page reports to host")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---
> drivers/virtio/virtio_balloon.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 53b4a3984e7d..6698edb61474 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/virtio.h>
> +#include <uapi/linux/virtio_ring.h>
> #include <linux/virtio_balloon.h>
> #include <linux/swap.h>
> #include <linux/workqueue.h>
> @@ -1175,6 +1176,11 @@ static int virtballoon_validate(struct virtio_device *vdev)
> else if (!virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON))
> __virtio_clear_bit(vdev, VIRTIO_BALLOON_F_REPORTING);
>
> + /*
> + * Disable indirect descriptors to avoid memory allocation in
> + * virtqueue_add during page reporting.
> + */
> + __virtio_clear_bit(vdev, VIRTIO_RING_F_INDIRECT_DESC);
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH net 0/2] vsock/virtio: fix MSG_PEEK calculation on bytes to copy
From: Michael S. Tsirkin @ 2026-06-15 19:48 UTC (permalink / raw)
To: Luigi Leonardi
Cc: Stefan Hajnoczi, Stefano Garzarella, Jason Wang, Xuan Zhuo,
Eugenio Pérez, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Arseniy Krasnov, kvm, virtualization,
netdev, linux-kernel
In-Reply-To: <20260402-fix_peek-v1-0-ad274fcef77b@redhat.com>
On Thu, Apr 02, 2026 at 10:18:00AM +0200, Luigi Leonardi wrote:
> `virtio_transport_stream_do_peek`, when calculating the number of bytes to copy,
> didn't consider the `offset`, caused by partial reads that happend before.
> This might cause out-of-bounds read that lead to an EFAULT.
> More details in the commit.
>
> Commit 1 introduces the fix
> Commit 2 introduces a test that checks for this bug to avoid future
> regressions.
>
> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> Luigi Leonardi (2):
> vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating bytes to copy
> vsock/test: add MSG_PEEK after partial recv test
>
> net/vmw_vsock/virtio_transport_common.c | 5 ++-
> tools/testing/vsock/vsock_test.c | 64 +++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 3 deletions(-)
> ---
> base-commit: 9147566d801602c9e7fc7f85e989735735bf38ba
> change-id: 20260401-fix_peek-6837b83469e3
>
> Best regards,
> --
> Luigi Leonardi <leonardi@redhat.com>
^ permalink raw reply
* Re: [PATCH net-next 0/2] selftests/vsock: improve vng version and quirk handling
From: patchwork-bot+netdevbpf @ 2026-06-15 20:00 UTC (permalink / raw)
To: Bobby Eshleman
Cc: sgarzare, shuah, virtualization, netdev, linux-kselftest,
linux-kernel, bobbyeshleman
In-Reply-To: <20260612-vsock-test-update-v1-0-7d7eeed3ac8f@meta.com>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 12 Jun 2026 12:08:40 -0700 you wrote:
> As vng has continued updating, there have been two things in our
> selftests that have been affected. One is that newer versions always
> emit the vng version warning, and two is that we have a workaround that
> is not needed in newer versions.
>
> This series just updates the version handling to allow all newer
> versions without warning and version-gates the workaround to only those
> versions that don't have the commit that fixed the root cause.
>
> [...]
Here is the summary with links:
- [net-next,1/2] selftests/vsock: accept vng 1.33 or >= 1.36
https://git.kernel.org/netdev/net-next/c/197503d5ac82
- [net-next,2/2] selftests/vsock: skip vng setsid workaround on >= 1.41
https://git.kernel.org/netdev/net-next/c/9361bff6bdb7
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v1] s390/virtio_ccw: Also suppress -EINVAL on device detach
From: William Bezenah @ 2026-06-15 20:01 UTC (permalink / raw)
To: Cornelia Huck, Halil Pasic
Cc: linux-s390, farman, hca, gor, agordeev, borntraeger, svens,
mjrosato, vneethv, oberpar, virtualization, kvm, linux-kernel
In-Reply-To: <875x3jn94r.fsf@redhat.com>
On 6/15/2026 10:58 AM, Cornelia Huck wrote:
> On Mon, Jun 15 2026, Halil Pasic <pasic@linux.ibm.com> wrote:
>
>> On Fri, 12 Jun 2026 17:54:07 +0200
>> William Bezenah <wbezenah@linux.ibm.com> wrote:
>>
>>> Since commit 8c58a229688c ("s390/cio: Do not unregister the
>>> subchannel based on DNV"), subchannel behavior following a device
>>> detach has been updated and results in -EINVAL being propagated
>>> rather than -ENODEV, originating from ccw_device_start_timeout_key()
>>> in cio/device_ops. In the end, the virtio driver has no ability to
>>> react to the difference between device and subchannel states here,
>>> and during detach, both -ENODEV and -EINVAL indicate the device
>>> cannot be used and should not be treated as errors requiring
>>> attention. Update error handling in virtio_ccw_del_vq() and
>>> virtio_ccw_drop_indicator() to suppress -EINVAL in addition to
>>> -ENODEV.
>> Hi William!
>>
>> Are you saying that ccw_device_start() started returning -EINVAL
>> since 8c58a229688c ("s390/cio: Do not unregister the subchannel based on
>> DNV")? Or did I somehow read the paragraph wrong?
>>
>> The funcition ccw_device_start is documented to return:
>> * Returns:
>> * %0, if the operation was successful;
>> * -%EBUSY, if the device is busy, or status pending;
>> * -%EACCES, if no path specified in @lpm is operational;
>> * -%ENODEV, if the device is not operational.
>> and the commit message does not say a thing about introducing -EINVAL to
>> the mix.
> The function may return -EINVAL for non-enabled subchannels
> (i.e. pmcw.ena == 0), maybe we get an all-zeroes schib with dnv == 0?
> I'd expect it not to be enabled in that case anyway.
Yep, that's at least how I've come to understand what changed. The
function ccw_device_start_timeout_key() has always returned -EINVAL
for non-enabled subchannels (pmcw.ena == 0), though it's not
documented in the header.
What changed with commit 8c58a229688c is that cio_update_schib() now
updates the schib even when DNV=0, rather than returning early as it
did previously. Somehow this update results in pmcw.ena == 0 in
ccw_device_start_timeout_key(). Previously, it saw pmcw.ena == 1 and
moved to the condition (cdev->private->state == DEV_STATE_NOT_OPER)
where it returned -ENODEV.
So the commit didn't introduce -EINVAL as a new return value, rather,
it changed the subchannel lifecycle such that existing paths now
propagate -EINVAL rather than -ENODEV during the device detach
scenario.
^ permalink raw reply
* Re: [PATCH v1] s390/virtio_ccw: Also suppress -EINVAL on device detach
From: Halil Pasic @ 2026-06-15 21:42 UTC (permalink / raw)
To: William Bezenah
Cc: Cornelia Huck, linux-s390, farman, hca, gor, agordeev,
borntraeger, svens, mjrosato, vneethv, oberpar, virtualization,
kvm, linux-kernel, Halil Pasic
In-Reply-To: <4d7fc371-4357-496f-9774-1f7a7c1a3091@linux.ibm.com>
On Mon, 15 Jun 2026 16:01:55 -0400
William Bezenah <wbezenah@linux.ibm.com> wrote:
> On 6/15/2026 10:58 AM, Cornelia Huck wrote:
> > On Mon, Jun 15 2026, Halil Pasic <pasic@linux.ibm.com> wrote:
> >
> >> On Fri, 12 Jun 2026 17:54:07 +0200
> >> William Bezenah <wbezenah@linux.ibm.com> wrote:
> >>
> >>> Since commit 8c58a229688c ("s390/cio: Do not unregister the
> >>> subchannel based on DNV"), subchannel behavior following a device
> >>> detach has been updated and results in -EINVAL being propagated
> >>> rather than -ENODEV, originating from ccw_device_start_timeout_key()
> >>> in cio/device_ops. In the end, the virtio driver has no ability to
> >>> react to the difference between device and subchannel states here,
> >>> and during detach, both -ENODEV and -EINVAL indicate the device
> >>> cannot be used and should not be treated as errors requiring
> >>> attention. Update error handling in virtio_ccw_del_vq() and
> >>> virtio_ccw_drop_indicator() to suppress -EINVAL in addition to
> >>> -ENODEV.
> >> Hi William!
> >>
> >> Are you saying that ccw_device_start() started returning -EINVAL
> >> since 8c58a229688c ("s390/cio: Do not unregister the subchannel based on
> >> DNV")? Or did I somehow read the paragraph wrong?
> >>
> >> The funcition ccw_device_start is documented to return:
> >> * Returns:
> >> * %0, if the operation was successful;
> >> * -%EBUSY, if the device is busy, or status pending;
> >> * -%EACCES, if no path specified in @lpm is operational;
> >> * -%ENODEV, if the device is not operational.
> >> and the commit message does not say a thing about introducing -EINVAL to
> >> the mix.
> > The function may return -EINVAL for non-enabled subchannels
> > (i.e. pmcw.ena == 0), maybe we get an all-zeroes schib with dnv == 0?
> > I'd expect it not to be enabled in that case anyway.
>
> Yep, that's at least how I've come to understand what changed. The
> function ccw_device_start_timeout_key() has always returned -EINVAL
> for non-enabled subchannels (pmcw.ena == 0), though it's not
> documented in the header.
Wasn't his -EINVAL actually introduced by commit:
823d494ac111 ("[S390] pm: ccw bus power management callbacks")?
>
> What changed with commit 8c58a229688c is that cio_update_schib() now
> updates the schib even when DNV=0, rather than returning early as it
> did previously. Somehow this update results in pmcw.ena == 0 in
> ccw_device_start_timeout_key(). Previously, it saw pmcw.ena == 1 and
> moved to the condition (cdev->private->state == DEV_STATE_NOT_OPER)
> where it returned -ENODEV.
Sounds fishy to me. As far as I understand the DNV takes precedence over
all other pieces of PMCW.
>
> So the commit didn't introduce -EINVAL as a new return value, rather,
> it changed the subchannel lifecycle such that existing paths now
> propagate -EINVAL rather than -ENODEV during the device detach
> scenario.
>
I'm not convinced returning -EINVAL in the given situation is the
right thing to do. Peter, would you mind to chime in?
Regards,
Halil
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] virtio_net: xsk: fix race in rx wake up
From: Menglong Dong @ 2026-06-16 1:48 UTC (permalink / raw)
To: menglong8.dong, Xuan Zhuo
Cc: mst, jasowang, andrew+netdev, davem, edumazet, kuba, pabeni,
minhquangbui99, kerneljasonxing, netdev, virtualization,
linux-kernel, eperezma
In-Reply-To: <1781491685.0613394-1-xuanzhuo@linux.alibaba.com>
On 2026/6/15 10:48 Xuan Zhuo <xuanzhuo@linux.alibaba.com> write:
> On Thu, 11 Jun 2026 10:56:43 +0800, menglong8.dong@gmail.com wrote:
> > From: Menglong Dong <dongml2@chinatelecom.cn>
> >
> > During packet receiving in virtio-net, the rq can be empty, which means
> > "rq->vq->num_free == virtqueue_get_vring_size(rq->vq)", in
> > virtnet_add_recvbuf_xsk(), if we are using xsk. Meanwhile, the fill ring
> > can be empty too, which means we can't allocate anything from
> > xsk_buff_alloc_batch(). Then, we will set the XDP_RING_NEED_WAKEUP flag.
> >
[...]
> >
> > + need_wakeup = xsk_uses_need_wakeup(pool);
> > xsk_buffs = rq->xsk_buffs;
> >
> > + /* If both rq->vq and fill ring are empty, and then the user submit
> > + * all the chunks to the fill ring and check the wake up flag
> > + * after xsk_buff_alloc_batch() and before xsk_set_rx_need_wakeup(),
> > + * we will lose the chance to wake up the rx napi, so we have to
> > + * set the need_wakeup flag here.
> > + */
> > + if (need_wakeup && virtqueue_get_vring_size(rq->vq) == rq->vq->num_free)
> > + xsk_set_rx_need_wakeup(pool);
>
> Is Condition A here too strict? We should trigger the wakeup under a wider range
> of scenarios.
Hi, Xuan. Thinks for your reviewing :)
The logic here is a addition logic to the origin wake up logic, which I planed
to fix a race condition. However, this race condition seems not likely to happen,
as we discussed in this thread:
https://lore.kernel.org/netdev/rHZz5_ylT4WggoZ-Ic2Q4w@linux.dev/
So this patch is not necessary, and I'll send the 2nd patch standalone.
Thanks!
Menglong Dong
>
> > +
> > num = xsk_buff_alloc_batch(pool, xsk_buffs, rq->vq->num_free);
> > if (!num) {
> > - if (xsk_uses_need_wakeup(pool)) {
> > + if (need_wakeup) {
> > xsk_set_rx_need_wakeup(pool);
> > /* Return 0 instead of -ENOMEM so that NAPI is
> > * descheduled.
> > @@ -1341,8 +1352,6 @@ static int virtnet_add_recvbuf_xsk(struct virtnet_info *vi, struct receive_queue
> > }
> >
> > return -ENOMEM;
> > - } else {
> > - xsk_clear_rx_need_wakeup(pool);
> > }
> >
> > len = xsk_pool_get_rx_frame_size(pool) + vi->hdr_len;
> > @@ -1363,6 +1372,16 @@ static int virtnet_add_recvbuf_xsk(struct virtnet_info *vi, struct receive_queue
> > goto err;
> > }
> >
> > + if (need_wakeup) {
> > + if (rq->vq->num_free)
> > + /* We have free buffers, so we'd better wake up the
> > + * rx napi as soon as possible.
> > + */
> > + xsk_set_rx_need_wakeup(pool);
>
> Is the purpose of waking up RX NAPI to invoke try_fill_recv? However,
> virtnet_poll does not call try_fill_recv directly. it is done
> conditionally.
>
> Thanks.
>
>
> > + else
> > + xsk_clear_rx_need_wakeup(pool);
> > + }
> > +
> > return num;
> >
> > err:
> > --
> > 2.54.0
> >
>
>
^ permalink raw reply
* Re:Re: [PATCH] virtio_net: disable cb when napi_schedule_prep fails during busy-poll
From: Lange Tang @ 2026-06-16 3:00 UTC (permalink / raw)
To: xuanzhuo@linux.alibaba.com, mst@redhat.com
Cc: edumazet@google.com, Jakub Kicinski,
virtualization@lists.linux.dev, Tang Longjun, jasowang@redhat.com
In-Reply-To: <1781517700.4206195-1-xuanzhuo@linux.alibaba.com>
At 2026-06-15 18:01:40, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
>On Mon, 15 Jun 2026 17:45:50 +0800, Longjun Tang <lange_tang@163.com> wrote:
>> From: Longjun Tang <tanglongjun@kylinos.cn>
>>
>> When busy-poll is active, napi_schedule_prep() returns false in
>> skb_recv_done(), so virtqueue_disable_cb() is skipped. The device
>> may keep firing irqs until the next poll round reaches
>> virtqueue_napi_complete(). If cb is enabled under busy-poll case,
>> it will lead to a large number of spurious interrupts. Explicitly
>> disable callbacks in this case to prevent spurious interrupts.
>>
>> Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
>> ---
>> drivers/net/virtio_net.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index f4adcfee7a80..6d675fddc59b 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -728,6 +728,8 @@ static void virtqueue_napi_schedule(struct napi_struct *napi,
>> if (napi_schedule_prep(napi)) {
>> virtqueue_disable_cb(vq);
>> __napi_schedule(napi);
>> + } else if (test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state)) {
>> + virtqueue_disable_cb(vq);
>
>I see, but we should avoid checking NAPI_STATE_IN_BUSY_POLL directly in the
>drivers. The NIC driver should remain agnostic to busy polling. I think we need
>a better way, maybe we should rewrite virtqueue_napi_schedule instead.
How about rewrite it like this?
static void virtqueue_napi_schedule(struct napi_struct *napi,
struct virtqueue *vq)
{
virtqueue_disable_cb(vq);
if (napi_schedule_prep(napi))
__napi_schedule(napi);
}
Any comments are welcome.
>
>Thanks.
>
>> }
>> }
>>
>> --
>> 2.25.1
>>
^ permalink raw reply
* Re:Re: [PATCH] virtio_net: disable cb when napi_schedule_prep fails during busy-poll
From: Xuan Zhuo @ 2026-06-16 3:27 UTC (permalink / raw)
To: Lange Tang
Cc: edumazet@google.com, Jakub Kicinski,
virtualization@lists.linux.dev, Tang Longjun, jasowang@redhat.com,
mst@redhat.com
In-Reply-To: <19692a81.3001.19ece5f8ddc.Coremail.lange_tang@163.com>
On Tue, 16 Jun 2026 11:00:29 +0800 (CST), Lange Tang <lange_tang@163.com> wrote:
> At 2026-06-15 18:01:40, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
> >On Mon, 15 Jun 2026 17:45:50 +0800, Longjun Tang <lange_tang@163.com> wrote:
> >> From: Longjun Tang <tanglongjun@kylinos.cn>
> >>
> >> When busy-poll is active, napi_schedule_prep() returns false in
> >> skb_recv_done(), so virtqueue_disable_cb() is skipped. The device
> >> may keep firing irqs until the next poll round reaches
> >> virtqueue_napi_complete(). If cb is enabled under busy-poll case,
> >> it will lead to a large number of spurious interrupts. Explicitly
> >> disable callbacks in this case to prevent spurious interrupts.
> >>
> >> Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
> >> ---
> >> drivers/net/virtio_net.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index f4adcfee7a80..6d675fddc59b 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -728,6 +728,8 @@ static void virtqueue_napi_schedule(struct napi_struct *napi,
> >> if (napi_schedule_prep(napi)) {
> >> virtqueue_disable_cb(vq);
> >> __napi_schedule(napi);
> >> + } else if (test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state)) {
> >> + virtqueue_disable_cb(vq);
> >
> >I see, but we should avoid checking NAPI_STATE_IN_BUSY_POLL directly in the
> >drivers. The NIC driver should remain agnostic to busy polling. I think we need
> >a better way, maybe we should rewrite virtqueue_napi_schedule instead.
>
> How about rewrite it like this?
> static void virtqueue_napi_schedule(struct napi_struct *napi,
> struct virtqueue *vq)
> {
> virtqueue_disable_cb(vq);
> if (napi_schedule_prep(napi))
> __napi_schedule(napi);
> }
> Any comments are welcome.
Another CPU could be running NAPI and has just enabled the callbacks (cb).
Meanwhile, this side unconditionally disables the cb. Since NAPI on the other
CPU hasn't exited yet, the subsequent prep on this side fails, leaving no one to
re-enable the cb.
Thanks.
> >
> >Thanks.
> >
> >> }
> >> }
> >>
> >> --
> >> 2.25.1
> >>
>
^ permalink raw reply
* [PATCH net v4] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-16 4:28 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
virtualization, linux-kernel, minhquangbui99, bestswngs,
Xiang Mei
receive_big() bounds the device-announced length by
(big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
add_recvbuf_big() sets sg[1] to start at offset
sizeof(struct padded_vnet_hdr) into the first page, so the chain
actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
check allows for the common hdr_len == 12 case.
A malicious virtio backend can announce a len in that gap. page_to_skb()
then walks one frag past the page chain, storing a NULL page->private
into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
write past the static frag array and a NULL frag handed up the rx path.
Bound len by the size add_recvbuf_big() actually advertised.
Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
v4: use easy to understand math to compute the max_len
v3: revoke 2/2 and add Xuan Zhuo's Reviewed-by tag
v2: add additiona check as 2/2
drivers/net/virtio_net.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f4adcfee7a80..8f4562316aaa 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1999,15 +1999,16 @@ static struct sk_buff *receive_big(struct net_device *dev,
struct virtnet_rq_stats *stats)
{
struct page *page = buf;
+ unsigned long max_len = (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE -
+ sizeof(struct padded_vnet_hdr) + vi->hdr_len;
struct sk_buff *skb;
/* Make sure that len does not exceed the size allocated in
* add_recvbuf_big.
*/
- if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
+ if (unlikely(len > max_len)) {
pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
- dev->name, len,
- (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
+ dev->name, len, max_len);
goto err;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net v3] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-16 4:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260614152904-mutt-send-email-mst@kernel.org>
On Sun, Jun 14, 2026 at 12:29 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sat, Jun 13, 2026 at 01:15:02PM -0700, Xiang Mei wrote:
> > On Wed, Jun 10, 2026 at 10:56 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Wed, Jun 10, 2026 at 07:46:16PM -0700, Xiang Mei wrote:
> > > > receive_big() bounds the device-announced length by
> > > > (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> > > > add_recvbuf_big() sets sg[1] to start at offset
> > > > sizeof(struct padded_vnet_hdr) into the first page, so the chain
> > > > actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> > > > big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> > > > check allows for the common hdr_len == 12 case.
> > > >
> > > > A malicious virtio backend can announce a len in that gap. page_to_skb()
> > > > then walks one frag past the page chain, storing a NULL page->private
> > > > into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> > > > write past the static frag array and a NULL frag handed up the rx path.
> > > >
> > > > Bound len by the size add_recvbuf_big() actually advertised.
> > > >
> > > > Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> > > > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > > > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > > > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > >
> > > Thanks for the patch! Something small to improve:
> > >
> > > > ---
> > > > v3: revoke 2/2 and add Xuan Zhuo's Reviewed-by tag
> > > >
> > > > drivers/net/virtio_net.c | 8 +++++---
> > > > 1 file changed, 5 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index f4adcfee7a80..afe73eda1491 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct net_device *dev,
> > > > struct virtnet_rq_stats *stats)
> > > > {
> > > > struct page *page = buf;
> > > > + unsigned long max_len;
> > >
> > > Assignment can happen here?
> > >
> > > > struct sk_buff *skb;
> > > >
> > > > /* Make sure that len does not exceed the size allocated in
> > > > * add_recvbuf_big.
> > > > */
> > > > - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> > > > + max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
> > > > + vi->big_packets_num_skbfrags * PAGE_SIZE;
> > >
> > > Took me a while to figure out what is going on, but I finally
> > > understand:
> > >
> > >
> > > Reducing
> > > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE
> > >
> > > (what we allocated)
> > >
> > > by sizeof(struct padded_vnet_hdr) - vi->hdr_len
> > >
> > >
> > > right?
> > >
> > > So clearer as:
> > >
> > >
> > > unsigned long max_len = (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE -
> > > sizeof(struct padded_vnet_hdr) + vi->hdr_len;
> > >
> > Right, that's the same value. Yours reads better!
> >
> > I'll fold this into the next respin. One thing I'd like to settle
> > first: David suggested storing this in a vi field computed once at the
> > probe (it's a per-device constant) and just comparing len against it
> > on the datapath, instead of re-deriving it in receive_big() each time.
> > I'll wait for his take on that and send a single v4 that covers both.
> >
> > Xiang
>
> I don't mind.
Thanks, Michael,
V4 has been sent.
Xiang
>
> > >
> > >
> > >
> > > > + if (unlikely(len > max_len)) {
> > > > pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> > > > - dev->name, len,
> > > > - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> > > > + dev->name, len, max_len);
> > > > goto err;
> > > > }
> > > >
> > > > --
> > > > 2.43.0
> > >
>
^ permalink raw reply
* Re: [PATCH net v4] virtio-net: fix len check in receive_big()
From: Michael S. Tsirkin @ 2026-06-16 4:39 UTC (permalink / raw)
To: Xiang Mei
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260616042837.2249468-1-xmei5@asu.edu>
On Mon, Jun 15, 2026 at 09:28:37PM -0700, Xiang Mei wrote:
> receive_big() bounds the device-announced length by
> (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> add_recvbuf_big() sets sg[1] to start at offset
> sizeof(struct padded_vnet_hdr) into the first page, so the chain
> actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> check allows for the common hdr_len == 12 case.
>
> A malicious virtio backend can announce a len in that gap. page_to_skb()
> then walks one frag past the page chain, storing a NULL page->private
> into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> write past the static frag array and a NULL frag handed up the rx path.
>
> Bound len by the size add_recvbuf_big() actually advertised.
>
> Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> v4: use easy to understand math to compute the max_len
> v3: revoke 2/2 and add Xuan Zhuo's Reviewed-by tag
I still feel 2/2 is good defence in depth but it can be
pursued separately.
> v2: add additiona check as 2/2
>
> drivers/net/virtio_net.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f4adcfee7a80..8f4562316aaa 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1999,15 +1999,16 @@ static struct sk_buff *receive_big(struct net_device *dev,
> struct virtnet_rq_stats *stats)
> {
> struct page *page = buf;
> + unsigned long max_len = (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE -
> + sizeof(struct padded_vnet_hdr) + vi->hdr_len;
> struct sk_buff *skb;
>
> /* Make sure that len does not exceed the size allocated in
> * add_recvbuf_big.
> */
> - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> + if (unlikely(len > max_len)) {
> pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> - dev->name, len,
> - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> + dev->name, len, max_len);
> goto err;
> }
>
> --
> 2.43.0
^ permalink raw reply
* Re: Patch "vsock/virtio: fix potential unbounded skb queue" has been added to the 6.6-stable tree
From: Greg KH @ 2026-06-16 4:47 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Sasha Levin, Michael S. Tsirkin, AVKrasnov, edumazet, eperezma,
jasowang, kuba, leonardi, stefanha, virtualization, xuanzhuo,
stable-commits, stable
In-Reply-To: <ag8EvTp29B-Q3nCq@sgarzare-redhat>
On Thu, May 21, 2026 at 03:15:54PM +0200, Stefano Garzarella wrote:
> On Sun, May 17, 2026 at 09:33:06AM -0400, Sasha Levin wrote:
> > > > What's the status of that fix?
> > >
> > > Stefano posted v3 and is working on v4.
> > >
> > > > Should it be reverted elsewhere?
> > >
> > > Donnu. With the change we have no DoS but the socket gets silently
> > > broken. Eric felt given the brokenness is upstream already it's better
> > > to work on a fix on top, not revert.
> >
> > Dropped from the 6.6, 6.12, 6.18, and 7.0 queues. We'll pick up Stefano's
> > follow-up once it lands upstream.
>
> FYI v4 is now merged in the net tree, so I guess they will land upstream
> soon. I CCed stable on both patches:
>
> a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
> c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full
> buf_alloc")
>
> Both are related, but the second is the main fix of this patch.
THe second one doesn't apply at all :(
^ permalink raw reply
* Re: [PATCH net v4] virtio-net: fix len check in receive_big()
From: Xiang Mei @ 2026-06-16 5:20 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel,
minhquangbui99, bestswngs
In-Reply-To: <20260616003903-mutt-send-email-mst@kernel.org>
On Mon, Jun 15, 2026 at 9:40 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Jun 15, 2026 at 09:28:37PM -0700, Xiang Mei wrote:
> > receive_big() bounds the device-announced length by
> > (big_packets_num_skbfrags + 1) * PAGE_SIZE. That is still too loose:
> > add_recvbuf_big() sets sg[1] to start at offset
> > sizeof(struct padded_vnet_hdr) into the first page, so the chain
> > actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> > big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> > check allows for the common hdr_len == 12 case.
> >
> > A malicious virtio backend can announce a len in that gap. page_to_skb()
> > then walks one frag past the page chain, storing a NULL page->private
> > into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> > write past the static frag array and a NULL frag handed up the rx path.
> >
> > Bound len by the size add_recvbuf_big() actually advertised.
> >
> > Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> > ---
> > v4: use easy to understand math to compute the max_len
> > v3: revoke 2/2 and add Xuan Zhuo's Reviewed-by tag
>
> I still feel 2/2 is good defence in depth but it can be
> pursued separately.
Thanks, Michael. I'll leave 2/2 out of this series.
Appreciate the review.
Xiang
>
> > v2: add additiona check as 2/2
> >
> > drivers/net/virtio_net.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index f4adcfee7a80..8f4562316aaa 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1999,15 +1999,16 @@ static struct sk_buff *receive_big(struct net_device *dev,
> > struct virtnet_rq_stats *stats)
> > {
> > struct page *page = buf;
> > + unsigned long max_len = (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE -
> > + sizeof(struct padded_vnet_hdr) + vi->hdr_len;
> > struct sk_buff *skb;
> >
> > /* Make sure that len does not exceed the size allocated in
> > * add_recvbuf_big.
> > */
> > - if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> > + if (unlikely(len > max_len)) {
> > pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> > - dev->name, len,
> > - (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> > + dev->name, len, max_len);
> > goto err;
> > }
> >
> > --
> > 2.43.0
>
^ permalink raw reply
* Re:Re:Re: [PATCH] virtio_net: disable cb when napi_schedule_prep fails during busy-poll
From: Lange Tang @ 2026-06-16 6:07 UTC (permalink / raw)
To: xuanzhuo@linux.alibaba.com, mst@redhat.com
Cc: edumazet@google.com, Jakub Kicinski,
virtualization@lists.linux.dev, Tang Longjun, jasowang@redhat.com
In-Reply-To: <1781580432.712892-1-xuanzhuo@linux.alibaba.com>
At 2026-06-16 11:27:12, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
>On Tue, 16 Jun 2026 11:00:29 +0800 (CST), Lange Tang <lange_tang@163.com> wrote:
>> At 2026-06-15 18:01:40, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
>> >On Mon, 15 Jun 2026 17:45:50 +0800, Longjun Tang <lange_tang@163.com> wrote:
>> >> From: Longjun Tang <tanglongjun@kylinos.cn>
>> >>
>> >> When busy-poll is active, napi_schedule_prep() returns false in
>> >> skb_recv_done(), so virtqueue_disable_cb() is skipped. The device
>> >> may keep firing irqs until the next poll round reaches
>> >> virtqueue_napi_complete(). If cb is enabled under busy-poll case,
>> >> it will lead to a large number of spurious interrupts. Explicitly
>> >> disable callbacks in this case to prevent spurious interrupts.
>> >>
>> >> Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
>> >> ---
>> >> drivers/net/virtio_net.c | 2 ++
>> >> 1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> >> index f4adcfee7a80..6d675fddc59b 100644
>> >> --- a/drivers/net/virtio_net.c
>> >> +++ b/drivers/net/virtio_net.c
>> >> @@ -728,6 +728,8 @@ static void virtqueue_napi_schedule(struct napi_struct *napi,
>> >> if (napi_schedule_prep(napi)) {
>> >> virtqueue_disable_cb(vq);
>> >> __napi_schedule(napi);
>> >> + } else if (test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state)) {
>> >> + virtqueue_disable_cb(vq);
>> >
>> >I see, but we should avoid checking NAPI_STATE_IN_BUSY_POLL directly in the
>> >drivers. The NIC driver should remain agnostic to busy polling. I think we need
>> >a better way, maybe we should rewrite virtqueue_napi_schedule instead.
>>
>> How about rewrite it like this?
>> static void virtqueue_napi_schedule(struct napi_struct *napi,
>> struct virtqueue *vq)
>> {
>> virtqueue_disable_cb(vq);
>> if (napi_schedule_prep(napi))
>> __napi_schedule(napi);
>> }
>> Any comments are welcome.
>
>
>Another CPU could be running NAPI and has just enabled the callbacks (cb).
>Meanwhile, this side unconditionally disables the cb. Since NAPI on the other
>CPU hasn't exited yet, the subsequent prep on this side fails, leaving no one to
>re-enable the cb.
>
>Thanks.
Regarding the case you described, when NAPI on another CPU exits, the virtqueue_napi_complete func
will be executed to re-enable cb. and if there is still unconsumed data in the virtqueue, virtqueue_napi_schedule
will be called again to schedule NAPI.
In summary, I think that the disable_cb and __napi_schedule within the virtqueue_napi_schedule func do not need to be bound together.
Any comments are welcome. Thinks.
>
>
>> >
>> >
>> >> }
>> >> }
>> >>
>> >> --
>> >> 2.25.1
>> >>
>>
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Miaohe Lin @ 2026-06-16 6:32 UTC (permalink / raw)
To: David Hildenbrand (Arm), Michael S. Tsirkin
Cc: Zi Yan, Andrew Morton, linux-kernel, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Hugh Dickins, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi
In-Reply-To: <be1b20ed-5b75-46b8-b7be-3c9b029f016b@kernel.org>
On 2026/6/15 18:54, David Hildenbrand (Arm) wrote:
> On 6/15/26 05:29, Miaohe Lin wrote:
>> On 2026/6/11 21:20, David Hildenbrand (Arm) wrote:
>>> On 6/11/26 09:36, Miaohe Lin wrote:
>>>>
>>>> Agree, it's not worth to do so.
>>>>
>>>>
>>>> Since memory_failure might be the only place, this change would be unacceptable.
>>>> We should come up with a better solution. Maybe we can try repeating SetPageHWPoison
>>>> and ClearPageHWPoison at a first attempt though it looks somewhat weird to me and makes
>>>> code more complicated.
>>>
>>> And I am fairly sure we could still have some remaining races ... it's shaky.
>>
>> I have to agree it's shaky.
>
> Right, just let writing task reschedule after reading the flags,
> but before writing the flags.
>
>> Any suggestion for next step?
>
> We have various code that assumes that no concurrent writes are
> possible, and consequently, we use no atomics.
>
> __free_pages_prepare() is just one user.
>
> Then we have __folio_set_locked(), __folio_clear_active()
> and __folio_clear_unevictable().
>
> But also __folio_mark_uptodate(), which is called rather frequently.
>
> page_cpupid_reset_last() is also a thing, but it mostly falls
> under __free_pages_prepare() handling.
>
> ... and __split_folio_to_order() also messes with flags directly without atomics.
>
>
> Many of these are only possible for frozen pages (refcount == 0). I think
> only __folio_set_locked() and __folio_mark_uptodate() are called on
> non-frozen pages, when there is the expectation that nobody will concurrently
> use atomics that would be bad (e.g., don't trylock if not an lru page).
>
Thanks David! This information is really helpful!
>
> We don't want to use atomics at these places just to please memory failure code.
Bad news. We have more places racing with memory failure code.
>
> Would it be sufficient to know in memory-failure code that concurrent
> handling succeeded?
I think so, that would be useful.
>
>
> Assume that we enlighten all non-atomics to grab the rcu read lock, such as
These non-atomics are defined and used because they want to avoid atomic ops overhead?
So I'm afraid using rcu read lock in these places would lead to unexpected overhead.
>
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 7223f6f4e2b4..3c3852b60bbd 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -803,10 +803,30 @@ static inline bool PageUptodate(const struct page *page)
> return folio_test_uptodate(page_folio(page));
> }
>
> +#ifdef CONFIG_MEMORY_FAILURE
> +static inline void page_flags_modify_nonatomic_begin(void)
> +{
> + rcu_read_lock();
> +}
> +static inline void page_flags_modify_nonatomic_end(void)
> +{
> + rcu_read_unlock();
> +}
> +#else
> +static inline void page_flags_modify_nonatomic_begin(void)
> +{
> +}
> +static inline void page_flags_modify_nonatomic_end(void)
> +{
> +}
> +#endif
> +
> static __always_inline void __folio_mark_uptodate(struct folio *folio)
> {
> smp_wmb();
> + page_flags_modify_nonatomic_begin();
> __set_bit(PG_uptodate, folio_flags(folio, 0));
> + page_flags_modify_nonatomic_end();
> }
>
>
> And then we have some retry logic such as:
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 51508a55c405..1123c40aaf43 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -162,6 +162,62 @@ static struct rb_root_cached pfn_space_itree = RB_ROOT_CACHED;
>
> static DEFINE_MUTEX(pfn_space_lock);
>
> +static bool page_test_set_hwpoison(struct page *page)
> +{
> + lockdep_assert_held(&mf_mutex);
> +
> + while (true) {
> + /* Already set -> not our problem. */
> + if (TestSetPageHWPoison(page))
> + return true;
> + /* Make sure concurrent non-atomic writers completed. */
> + synchronize_rcu();
> + /* Setting the flag was sticky. */
> + if (PageHWPoison(page))
> + return false;
> + }
> +}
> +
> +static bool page_test_clear_hwpoison(struct page *page)
> +{
> + lockdep_assert_held(&mf_mutex);
> +
> + while (true) {
> + /* Already clear -> not our problem. */
> + if (!TestClearPageHWPoison(page))
> + return false;
> + /* Make sure concurrent non-atomic writers completed. */
> + synchronize_rcu();
> + /* Clearing the flag was sticky. */
> + if (!PageHWPoison(page))
> + return true;
> + }
> +}
> +
> +static void page_set_hwpoison(struct page *page)
> +{
> + lockdep_assert_held(&mf_mutex);
> +
> + while (!PageHWPoison(page)) {
> + SetPageHWPoison(page);
> +
> + /* Make sure concurrent non-atomic writers completed. */
> + synchronize_rcu();
> + }
> +}
> +
> +static void page_clear_hwpoison(struct page *page)
> +{
> + lockdep_assert_held(&mf_mutex);
> +
> + while (PageHWPoison(page)) {
> + ClearPageHWPoison(page);
> +
> + /* Make sure concurrent non-atomic writers completed. */
> + synchronize_rcu();
> + }
> +}
> +
> /*
> * Return values:
> * 1: the page is dissolved (if needed) and taken off from buddy,
> @@ -199,7 +255,7 @@ static bool page_handle_poison(struct page *page, bool hugepage_or_freepage, boo
> return false;
> }
>
> - SetPageHWPoison(page);
> + page_set_hwpoison(page);
> if (release)
> put_page(page);
> page_ref_inc(page);
> @@ -1744,7 +1800,7 @@ static int mf_generic_kill_procs(unsigned long long pfn, int flags,
> * Use this flag as an indication that the dax page has been
> * remapped UC to prevent speculative consumption of poison.
> */
> - SetPageHWPoison(&folio->page);
> + page_set_hwpoison(&folio->page);
>
> /*
> * Unlike System-RAM there is no possibility to swap in a
> @@ -1789,7 +1845,7 @@ int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
> goto unlock;
>
> if (!pre_remove)
> - SetPageHWPoison(page);
> + page_set_hwpoison(page);
>
> /*
> * The pre_remove case is revoking access, the memory is still
> @@ -1866,7 +1922,7 @@ static unsigned long __folio_free_raw_hwp(struct folio *folio, bool move_flag)
> head = llist_del_all(raw_hwp_list_head(folio));
> llist_for_each_entry_safe(p, next, head, node) {
> if (move_flag)
> - SetPageHWPoison(p->page);
> + page_set_hwpoison(p->page);
> else
> num_poisoned_pages_sub(page_to_pfn(p->page), 1);
> kfree(p);
> @@ -2380,7 +2436,7 @@ int memory_failure(unsigned long pfn, int flags)
> if (res != -ENOENT)
> goto unlock_mutex;
>
> - if (TestSetPageHWPoison(p)) {
> + if (page_test_set_hwpoison(p)) {
> res = -EHWPOISON;
> if (flags & MF_ACTION_REQUIRED)
> res = kill_accessing_process(current, pfn, flags);
> @@ -2410,7 +2466,7 @@ int memory_failure(unsigned long pfn, int flags)
> } else {
> /* We lost the race, try again */
> if (retry) {
> - ClearPageHWPoison(p);
> + page_clear_hwpoison(p);
> retry = false;
> goto try_again;
> }
> @@ -2431,7 +2487,7 @@ int memory_failure(unsigned long pfn, int flags)
> /* filter pages that are protected from hwpoison test by users */
> folio_lock(folio);
> if (hwpoison_filter(p)) {
> - ClearPageHWPoison(p);
> + page_clear_hwpoison(p);
> folio_unlock(folio);
> folio_put(folio);
> res = -EOPNOTSUPP;
> @@ -2751,7 +2807,7 @@ int unpoison_memory(unsigned long pfn)
> }
>
> folio_put(folio);
> - if (TestClearPageHWPoison(p)) {
> + if (page_test_clear_hwpoison(p)) {
> folio_put(folio);
> ret = 0;
> }
>
>
> Maybe that would work. There would still be issues to solve
>
> (a) We don't hold the mf_mutex on all call paths, but we really need it so a
> page_test_set_hwpoison() cannot race in weird ways with the other primitives I think.
>
> (b) There are some leftover SetPageHWPoison etc. instances. The ones in
> arch/x86/kernel/cpu/mce/core.c likely cannot grab the mutex, but maybe they are
> corner cases either way and we can document the situation.
>
>
> Further, while I assume the synchronize_rcu() on the MCE path should be fine
> (who cares about performance there?), I don't know if the added RCU read lock
> on some paths could be noticable.
>
> So one idea worth discussing, but I am sure there are more problems.
I think this is a good idea, although there are some remaining issues.
But such race should be really rare, is it worth all this effort? Could we
simply aim to resolve, not to be flawless? I.e. could we simply check
and re-set the hwpoison flag at the end of memory_failure handling to
simply avoid losing hwpoison flag as a best-effort attempt? Would it be
acceptable?
Thanks.
.
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: David Hildenbrand (Arm) @ 2026-06-16 6:56 UTC (permalink / raw)
To: Miaohe Lin, Michael S. Tsirkin
Cc: Zi Yan, Andrew Morton, linux-kernel, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Baolin Wang, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Hugh Dickins, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi
In-Reply-To: <d7e97bec-72bf-7d56-3811-6d9c41fefb35@huawei.com>
>>
>>
>> Assume that we enlighten all non-atomics to grab the rcu read lock, such as
>
> These non-atomics are defined and used because they want to avoid atomic ops overhead?
> So I'm afraid using rcu read lock in these places would lead to unexpected overhead.
It should be cheaper than atomics IIUC. Further, I assume that some pages could
batch over multiple such operations (esp. page freeing path when we process tail
pages).
With !CONFIG_PREEMPT_RCU it's simply preempt_disable()/preempt_enable(), which
is either a NOP or just adjusting the preempt counter of the current thread. Cheap.
With CONFIG_PREEMPT_RCU we mostly increment current->rcu_read_lock_nesting. But
there might be a function call involved (did not look into the details). So that
variant should be slightly more expensive.
We'd have to measure what an addition rcu read lock would cost in there. that
should be fairly easy to benchmark.
>>
>> Maybe that would work. There would still be issues to solve
>>
>> (a) We don't hold the mf_mutex on all call paths, but we really need it so a
>> page_test_set_hwpoison() cannot race in weird ways with the other primitives I think.
>>
>> (b) There are some leftover SetPageHWPoison etc. instances. The ones in
>> arch/x86/kernel/cpu/mce/core.c likely cannot grab the mutex, but maybe they are
>> corner cases either way and we can document the situation.
>>
>>
>> Further, while I assume the synchronize_rcu() on the MCE path should be fine
>> (who cares about performance there?), I don't know if the added RCU read lock
>> on some paths could be noticable.
>>
>> So one idea worth discussing, but I am sure there are more problems.
>
> I think this is a good idea, although there are some remaining issues.
> But such race should be really rare, is it worth all this effort? Could we
> simply aim to resolve, not to be flawless? I.e. could we simply check
> and re-set the hwpoison flag at the end of memory_failure handling to
> simply avoid losing hwpoison flag as a best-effort attempt? Would it be
> acceptable?
Hacky. Sufficient for the hypervisor to suspend the nonatomic-setting CPU at the
wrong time to still trigger the same behavior.
I think, either we fix it properly, or we redesign hwpoison handling to deal
with setting/clearing becoming stale at some random point in the future.
--
Cheers,
David
^ permalink raw reply
* Re:Re:Re: [PATCH] virtio_net: disable cb when napi_schedule_prep fails during busy-poll
From: Xuan Zhuo @ 2026-06-16 6:49 UTC (permalink / raw)
To: Lange Tang
Cc: edumazet@google.com, Jakub Kicinski,
virtualization@lists.linux.dev, Tang Longjun, jasowang@redhat.com,
mst@redhat.com
In-Reply-To: <4118686.4d22.19ecf0ad78e.Coremail.lange_tang@163.com>
On Tue, 16 Jun 2026 14:07:34 +0800 (CST), Lange Tang <lange_tang@163.com> wrote:
> At 2026-06-16 11:27:12, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
> >On Tue, 16 Jun 2026 11:00:29 +0800 (CST), Lange Tang <lange_tang@163.com> wrote:
> >> At 2026-06-15 18:01:40, "Xuan Zhuo" <xuanzhuo@linux.alibaba.com> wrote:
> >> >On Mon, 15 Jun 2026 17:45:50 +0800, Longjun Tang <lange_tang@163.com> wrote:
> >> >> From: Longjun Tang <tanglongjun@kylinos.cn>
> >> >>
> >> >> When busy-poll is active, napi_schedule_prep() returns false in
> >> >> skb_recv_done(), so virtqueue_disable_cb() is skipped. The device
> >> >> may keep firing irqs until the next poll round reaches
> >> >> virtqueue_napi_complete(). If cb is enabled under busy-poll case,
> >> >> it will lead to a large number of spurious interrupts. Explicitly
> >> >> disable callbacks in this case to prevent spurious interrupts.
> >> >>
> >> >> Signed-off-by: Longjun Tang <tanglongjun@kylinos.cn>
> >> >> ---
> >> >> drivers/net/virtio_net.c | 2 ++
> >> >> 1 file changed, 2 insertions(+)
> >> >>
> >> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> >> index f4adcfee7a80..6d675fddc59b 100644
> >> >> --- a/drivers/net/virtio_net.c
> >> >> +++ b/drivers/net/virtio_net.c
> >> >> @@ -728,6 +728,8 @@ static void virtqueue_napi_schedule(struct napi_struct *napi,
> >> >> if (napi_schedule_prep(napi)) {
> >> >> virtqueue_disable_cb(vq);
> >> >> __napi_schedule(napi);
> >> >> + } else if (test_bit(NAPI_STATE_IN_BUSY_POLL, &napi->state)) {
> >> >> + virtqueue_disable_cb(vq);
> >> >
> >> >I see, but we should avoid checking NAPI_STATE_IN_BUSY_POLL directly in the
> >> >drivers. The NIC driver should remain agnostic to busy polling. I think we need
> >> >a better way, maybe we should rewrite virtqueue_napi_schedule instead.
> >>
> >> How about rewrite it like this?
> >> static void virtqueue_napi_schedule(struct napi_struct *napi,
> >> struct virtqueue *vq)
> >> {
> >> virtqueue_disable_cb(vq);
> >> if (napi_schedule_prep(napi))
> >> __napi_schedule(napi);
> >> }
> >> Any comments are welcome.
> >
> >
> >Another CPU could be running NAPI and has just enabled the callbacks (cb).
> >Meanwhile, this side unconditionally disables the cb. Since NAPI on the other
> >CPU hasn't exited yet, the subsequent prep on this side fails, leaving no one to
> >re-enable the cb.
> >
> >Thanks.
>
> Regarding the case you described, when NAPI on another CPU exits, the virtqueue_napi_complete func
> will be executed to re-enable cb. and if there is still unconsumed data in the virtqueue, virtqueue_napi_schedule
> will be called again to schedule NAPI.
>
> In summary, I think that the disable_cb and __napi_schedule within the virtqueue_napi_schedule func do not need to be bound together.
>
> Any comments are welcome. Thinks.
<Your code>
static void virtqueue_napi_schedule(struct napi_struct *napi,
struct virtqueue *vq)
{
|static bool virtqueue_napi_complete(struct napi_struct *napi,
| struct virtqueue *vq, int processed)
|{
| int opaque;
|
| opaque = virtqueue_enable_cb_prepare(vq);
|
virtqueue_disable_cb(vq); |
if (napi_schedule_prep(napi)) |
__napi_schedule(napi); |
| if (napi_complete_done(napi, processed)) {
| if (unlikely(virtqueue_poll(vq, opaque)))
| virtqueue_napi_schedule(napi, vq);
| else
| return true; // return directly
| } else {
| virtqueue_disable_cb(vq);
| }
|
| return false;
|}
}
1. new packets (notified by irq) are consumed by napi before virtqueue_napi_complete
2. poll is not called by irq, maybe xsk wake up. So irq is not disabled.
Thanks.
>
> >
> >
> >> >
> >> >
> >> >> }
> >> >> }
> >> >>
> >> >> --
> >> >> 2.25.1
> >> >>
> >>
>
^ permalink raw reply
* Re: Patch "vsock/virtio: fix potential unbounded skb queue" has been added to the 6.6-stable tree
From: Stefano Garzarella @ 2026-06-16 7:52 UTC (permalink / raw)
To: Greg KH
Cc: Sasha Levin, Michael S. Tsirkin, AVKrasnov, edumazet, eperezma,
jasowang, kuba, leonardi, stefanha, virtualization, xuanzhuo,
stable-commits, stable
In-Reply-To: <2026061624-harbor-capture-a5bf@gregkh>
On Tue, Jun 16, 2026 at 10:17:31AM +0530, Greg KH wrote:
>On Thu, May 21, 2026 at 03:15:54PM +0200, Stefano Garzarella wrote:
>> On Sun, May 17, 2026 at 09:33:06AM -0400, Sasha Levin wrote:
>> > > > What's the status of that fix?
>> > >
>> > > Stefano posted v3 and is working on v4.
>> > >
>> > > > Should it be reverted elsewhere?
>> > >
>> > > Donnu. With the change we have no DoS but the socket gets silently
>> > > broken. Eric felt given the brokenness is upstream already it's better
>> > > to work on a fix on top, not revert.
>> >
>> > Dropped from the 6.6, 6.12, 6.18, and 7.0 queues. We'll pick up Stefano's
>> > follow-up once it lands upstream.
>>
>> FYI v4 is now merged in the net tree, so I guess they will land upstream
>> soon. I CCed stable on both patches:
>>
>> a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
>> c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full
>> buf_alloc")
>>
>> Both are related, but the second is the main fix of this patch.
>
>THe second one doesn't apply at all :(
>
The second one is the fix of the patch originally added to stable queue
by this thread, so should be applied on top of it (commit 059b7dbd20a6
("vsock/virtio: fix potential unbounded skb queue")).
I'm working on improving memory management, but for now I think it makes
sense to backport all three to the stable branches.
So, in summary:
059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
a4f0b001782b ("vsock/virtio: reset connection on receiving queue overflow")
c6087c5aaad6 ("vsock/virtio: fix skb overhead accounting to preserve full buf_alloc")
Thanks,
Stefano
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox