* Re: [PATCH] [v3] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO
From: Michael S. Tsirkin @ 2026-06-10 5:26 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jason Wang, Xie Yongji, Arnd Bergmann, Eugenio Pérez,
Xuan Zhuo, Marco Crivellari, Anders Roxell, virtualization,
linux-kernel
In-Reply-To: <20260213154051.4172275-1-arnd@kernel.org>
On Fri, Feb 13, 2026 at 04:40:46PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> These two ioctls are incompatible on 32-bit x86 userspace, because
> the data structures are shorter than they are on 64-bit.
>
> Add a proper .compat_ioctl handler for x86 that reads the structures
> with the smaller padding before calling the internal handlers. On
> all other architectures, CONFIG_COMPAT_FOR_U64_ALIGNMENT is disabled
> and no special handling is required.
>
> Fixes: ad146355bfad ("vduse: Support querying information of IOVA regions")
> Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
> Acked-by: Eugenio Pérez <eperezma@redhat.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v3 changes:
> - check CONFIG_COMPAT_FOR_U64_ALIGNMENT in preprocessor
> v2 changes:
> - split compat handler into separate function
> ---
> drivers/vdpa/vdpa_user/vduse_dev.c | 123 ++++++++++++++++++++++++++++-
> 1 file changed, 122 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 405d59610f76..e0f5a7397221 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1618,6 +1618,127 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> return ret;
> }
>
> +#ifdef CONFIG_COMPAT_FOR_U64_ALIGNMENT
> +/*
> + * i386 has different alignment constraints than x86_64,
> + * so there are only 3 bytes of padding instead of 7.
> + */
> +struct compat_vduse_iotlb_entry {
> + compat_u64 offset;
> + compat_u64 start;
> + compat_u64 last;
> + __u8 perm;
> + __u8 padding[3];
> +};
> +#define COMPAT_VDUSE_IOTLB_GET_FD _IOWR(VDUSE_BASE, 0x10, struct compat_vduse_iotlb_entry)
> +
> +struct compat_vduse_vq_info {
> + __u32 index;
> + __u32 num;
> + compat_u64 desc_addr;
> + compat_u64 driver_addr;
> + compat_u64 device_addr;
> + union {
> + struct vduse_vq_state_split split;
> + struct vduse_vq_state_packed packed;
> + };
> + __u8 ready;
> + __u8 padding[3];
> +} __uapi_arch_align;
what is this __uapi_arch_align supposed to be doing?
It compiles by luck because gcc thinks it's a global variable.
> +#define COMPAT_VDUSE_VQ_GET_INFO _IOWR(VDUSE_BASE, 0x15, struct compat_vduse_vq_info)
> +
> +static long vduse_dev_compat_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct vduse_dev *dev = file->private_data;
> + void __user *argp = (void __user *)arg;
> + int ret;
> +
> + if (unlikely(dev->broken))
> + return -EPERM;
> +
> + switch (cmd) {
> + case COMPAT_VDUSE_IOTLB_GET_FD: {
> + struct vduse_iotlb_entry_v2 entry = {0};
> + struct file *f = NULL;
> +
> + ret = -EFAULT;
> + if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))
> + break;
> +
> + ret = vduse_dev_iotlb_entry(dev, &entry, &f, NULL);
> + if (ret)
> + break;
> +
> + ret = -EINVAL;
> + if (!f)
> + break;
> +
> + ret = copy_to_user(argp, &entry, _IOC_SIZE(cmd));
> + if (ret) {
> + ret = -EFAULT;
> + fput(f);
> + break;
> + }
> + ret = receive_fd(f, NULL, perm_to_file_flags(entry.perm));
> + fput(f);
> + break;
> + }
> + case COMPAT_VDUSE_VQ_GET_INFO: {
> + struct vduse_vq_info vq_info = {};
> + struct vduse_virtqueue *vq;
> + u32 index;
> +
> + ret = -EFAULT;
> + if (copy_from_user(&vq_info, argp,
> + sizeof(struct compat_vduse_vq_info)))
> + break;
> +
> + ret = -EINVAL;
> + if (vq_info.index >= dev->vq_num)
> + break;
> +
> + index = array_index_nospec(vq_info.index, dev->vq_num);
> + vq = dev->vqs[index];
> + vq_info.desc_addr = vq->desc_addr;
> + vq_info.driver_addr = vq->driver_addr;
> + vq_info.device_addr = vq->device_addr;
> + vq_info.num = vq->num;
> +
> + if (dev->driver_features & BIT_ULL(VIRTIO_F_RING_PACKED)) {
> + vq_info.packed.last_avail_counter =
> + vq->state.packed.last_avail_counter;
> + vq_info.packed.last_avail_idx =
> + vq->state.packed.last_avail_idx;
> + vq_info.packed.last_used_counter =
> + vq->state.packed.last_used_counter;
> + vq_info.packed.last_used_idx =
> + vq->state.packed.last_used_idx;
> + } else
> + vq_info.split.avail_index =
> + vq->state.split.avail_index;
> +
> + vq_info.ready = vq->ready;
> +
> + ret = -EFAULT;
> + if (copy_to_user(argp, &vq_info,
> + sizeof(struct compat_vduse_vq_info)))
> + break;
> +
> + ret = 0;
> + break;
> + }
> + default:
> + ret = -ENOIOCTLCMD;
> + break;
> + }
> +
> + return vduse_dev_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> +}
> +#else
> +#define vduse_dev_compat_ioctl compat_ptr_ioctl
> +#endif
> +
> static int vduse_dev_release(struct inode *inode, struct file *file)
> {
> struct vduse_dev *dev = file->private_data;
> @@ -1678,7 +1799,7 @@ static const struct file_operations vduse_dev_fops = {
> .write_iter = vduse_dev_write_iter,
> .poll = vduse_dev_poll,
> .unlocked_ioctl = vduse_dev_ioctl,
> - .compat_ioctl = compat_ptr_ioctl,
> + .compat_ioctl = vduse_dev_compat_ioctl,
> .llseek = noop_llseek,
> };
>
> --
> 2.39.5
^ permalink raw reply
* Re: [PATCH splitout] mm: page_reporting: allow driver to set batch capacity
From: Michael S. Tsirkin @ 2026-06-09 22:00 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, Miaohe Lin, David Hildenbrand (Arm), Jason Wang,
Xuan Zhuo, Eugenio Pérez, Muchun Song, Oscar Salvador,
Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi,
Alexander Duyck
In-Reply-To: <aiiJPwhn6gF3ILur@gourry-fedora-PF4VCD3F>
On Tue, Jun 09, 2026 at 05:44:31PM -0400, Gregory Price wrote:
> On Tue, Jun 09, 2026 at 04:08:08PM -0400, Michael S. Tsirkin wrote:
> > On Tue, Jun 09, 2026 at 01:44:37PM -0400, Gregory Price wrote:
> > > On Tue, Jun 09, 2026 at 11:53:20AM -0400, Michael S. Tsirkin wrote:
> > > > --- a/mm/page_reporting.c
> > > > +++ b/mm/page_reporting.c
> > > > @@ -174,10 +174,10 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
> > > > * 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.
> > > > + * The division here uses integer division; capacity need
> > > > + * not 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);
> > > >
> > >
> > > Initial look - is there a div-by-0 here? I noticed the old check
> > > prevents this from being (0 * 16), but i don't see (on first pass)
> > > the same check anywhere.
> > >
> > > Unless this line below always forces the above to be a
> > > PAGE_REPORTING_CAPCAITY if it's set to 0.
> >
> > It does, does it not?
> >
> > > > + if (!prdev->capacity || prdev->capacity > PAGE_REPORTING_CAPACITY)
> > > > + prdev->capacity = PAGE_REPORTING_CAPACITY;
> > > > +
> > >
> > > It's worth making this corner condition a little more obvious.
> > >
> > > The code intends for
> > >
> > > if (capacity == 0)
> > > capacity = PAGE_REPORTING_CAPACITY
> > >
> > > but that's not reflected in the changelog as a default value.
> > >
> > > When happens if a driver sets (capacity=0) either on purpose (???)
> >
> > what would the purpose be? if you don't want reporting do not register.
> >
> > > or
> > > because there's a bug (???)
> >
> > exactly ??? since where are we practicing defensive programming in kernel
> > APIs?
> >
> > > and then page_reporting.c forces it up to
> > > 32?
> > >
> > > There's something to improve here.
> > >
> > > ~Gregory
> >
> >
> > So I'll update the commit log to mention PAGE_REPORTING_CAPACITY.
> > And maybe a comment near capacity field?
> > Should be enough?
>
> I suppose the question is whether capacity=0 should cause a WARN (i.e.
> only a bug explains that value), or if capacity=0 means something
> special (i.e. use the default) and therefore that should be documented.
> I don't know which of these is the case, but if it's the latter than
> that deserves a comment yes.
>
> ~Gregory
It's the default. Will document.
^ permalink raw reply
* Re: [PATCH splitout] mm: page_reporting: allow driver to set batch capacity
From: Gregory Price @ 2026-06-09 21:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Miaohe Lin, David Hildenbrand (Arm), Jason Wang,
Xuan Zhuo, Eugenio Pérez, Muchun Song, Oscar Salvador,
Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi,
Alexander Duyck
In-Reply-To: <20260609160506-mutt-send-email-mst@kernel.org>
On Tue, Jun 09, 2026 at 04:08:08PM -0400, Michael S. Tsirkin wrote:
> On Tue, Jun 09, 2026 at 01:44:37PM -0400, Gregory Price wrote:
> > On Tue, Jun 09, 2026 at 11:53:20AM -0400, Michael S. Tsirkin wrote:
> > > --- a/mm/page_reporting.c
> > > +++ b/mm/page_reporting.c
> > > @@ -174,10 +174,10 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
> > > * 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.
> > > + * The division here uses integer division; capacity need
> > > + * not 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);
> > >
> >
> > Initial look - is there a div-by-0 here? I noticed the old check
> > prevents this from being (0 * 16), but i don't see (on first pass)
> > the same check anywhere.
> >
> > Unless this line below always forces the above to be a
> > PAGE_REPORTING_CAPCAITY if it's set to 0.
>
> It does, does it not?
>
> > > + if (!prdev->capacity || prdev->capacity > PAGE_REPORTING_CAPACITY)
> > > + prdev->capacity = PAGE_REPORTING_CAPACITY;
> > > +
> >
> > It's worth making this corner condition a little more obvious.
> >
> > The code intends for
> >
> > if (capacity == 0)
> > capacity = PAGE_REPORTING_CAPACITY
> >
> > but that's not reflected in the changelog as a default value.
> >
> > When happens if a driver sets (capacity=0) either on purpose (???)
>
> what would the purpose be? if you don't want reporting do not register.
>
> > or
> > because there's a bug (???)
>
> exactly ??? since where are we practicing defensive programming in kernel
> APIs?
>
> > and then page_reporting.c forces it up to
> > 32?
> >
> > There's something to improve here.
> >
> > ~Gregory
>
>
> So I'll update the commit log to mention PAGE_REPORTING_CAPACITY.
> And maybe a comment near capacity field?
> Should be enough?
I suppose the question is whether capacity=0 should cause a WARN (i.e.
only a bug explains that value), or if capacity=0 means something
special (i.e. use the default) and therefore that should be documented.
I don't know which of these is the case, but if it's the latter than
that deserves a comment yes.
~Gregory
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Michael S. Tsirkin @ 2026-06-09 21:00 UTC (permalink / raw)
To: Zi Yan
Cc: Miaohe Lin, David Hildenbrand (Arm), 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: <4BA276D9-9EB9-4E2A-8A05-657ACACFF227@nvidia.com>
On Tue, Jun 09, 2026 at 04:54:01PM -0400, Zi Yan wrote:
> On 9 Jun 2026, at 16:34, Michael S. Tsirkin wrote:
>
> > On Tue, Jun 09, 2026 at 02:52:47PM -0400, Zi Yan wrote:
> >> On 9 Jun 2026, at 14:39, Zi Yan wrote:
> >>
> >>> On 9 Jun 2026, at 14:38, David Hildenbrand (Arm) wrote:
> >>>
> >>>> On 6/9/26 20:10, Andrew Morton wrote:
> >>>>> On Tue, 9 Jun 2026 06:12:49 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >>>>>
> >>>>>> TestSetPageHWPoison() is called without zone->lock, so its atomic
> >>>>>> update to page->flags can race with non-atomic flag operations
> >>>>>> that run under zone->lock in the buddy allocator.
> >>>>>>
> >>>>>> In particular, __free_pages_prepare() does:
> >>>>>>
> >>>>>> page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
> >>>>>>
> >>>>>> This non-atomic read-modify-write, while correctly excluding
> >>>>>> __PG_HWPOISON from the mask, can still lose a concurrent
> >>>>>> TestSetPageHWPoison if the read happens before the poison bit
> >>>>>> is set and the write happens after. Will only get worse if/when
> >>>>>> we add more non-atomic flag operations.
> >>>>>>
> >>>>>> Fix by acquiring zone->lock around TestSetPageHWPoison and
> >>>>>> around ClearPageHWPoison in the retry path. This
> >>>>>> serializes with all buddy flag manipulation. The cost is
> >>>>>> negligible: one lock/unlock in an extremely rare path
> >>>>>> (hardware memory errors).
> >>>>>>
> >>>>>> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
> >>>>>> in this file operate on pages already removed from the buddy
> >>>>>> allocator or on non-buddy pages (DAX, hugetlb), so they do not
> >>>>>> need zone->lock protection.
> >>>>>
> >>>>> Sashiko is saying this doesn't do anything "Because
> >>>>> __free_pages_prepare() executes entirely locklessly". Did it goof?
> >>>>>
> >>>>> https://sashiko.dev/#/patchset/df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com
> >>>>
> >>>> Battle of the bots: it's right.
> >>>
> >>> Yep, __free_pages_prepare() changes the page flag without holding
> >>> zone->lock.
> >>
> >> __free_pages_prepare() works on frozen pages and assumes no one else
> >> touches the input page. To avoid this race, memory_failure() might
> >> want to try_get_page() before TestClearPageHWPoison(), but I am not
> >> sure if that works along with memory failure flow.
> >>
> >> Best Regards,
> >> Yan, Zi
> >
> >
> >
> > Actually memory failure already plays with this down the road no?
> >
> > So maybe it's enough to just SetPageHWPoison afterwards again?
> >
> >
> > diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> > index ee42d4361309..4758fea94a96 100644
> > --- a/mm/memory-failure.c
> > +++ b/mm/memory-failure.c
> > @@ -2415,6 +2415,7 @@ int memory_failure(unsigned long pfn, int flags)
> > if (!res) {
> > if (is_free_buddy_page(p)) {
> > if (take_page_off_buddy(p)) {
> > + SetPageHWPoison(p);
> > page_ref_inc(p);
> > res = MF_RECOVERED;
> > } else {
> >
> >
> > and maybe in a bunch of other places in there?
>
> You mean for fear of losing HWPoison flag in the earlier TestSetPageHWPoison(),
> just set it again here?
Yea.
> Why not do it after get_hwpoison_page(), since that
> is the expected page flag?
It's still in the buddy at that point right? I'm worried buddy might
poke at flags.
> Miaohe probably can give a better answer here.
>
>
> Best Regards,
> Yan, Zi
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Zi Yan @ 2026-06-09 20:54 UTC (permalink / raw)
To: Michael S. Tsirkin, Miaohe Lin
Cc: David Hildenbrand (Arm), 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: <20260609162437-mutt-send-email-mst@kernel.org>
On 9 Jun 2026, at 16:34, Michael S. Tsirkin wrote:
> On Tue, Jun 09, 2026 at 02:52:47PM -0400, Zi Yan wrote:
>> On 9 Jun 2026, at 14:39, Zi Yan wrote:
>>
>>> On 9 Jun 2026, at 14:38, David Hildenbrand (Arm) wrote:
>>>
>>>> On 6/9/26 20:10, Andrew Morton wrote:
>>>>> On Tue, 9 Jun 2026 06:12:49 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>>>
>>>>>> TestSetPageHWPoison() is called without zone->lock, so its atomic
>>>>>> update to page->flags can race with non-atomic flag operations
>>>>>> that run under zone->lock in the buddy allocator.
>>>>>>
>>>>>> In particular, __free_pages_prepare() does:
>>>>>>
>>>>>> page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>>>>>>
>>>>>> This non-atomic read-modify-write, while correctly excluding
>>>>>> __PG_HWPOISON from the mask, can still lose a concurrent
>>>>>> TestSetPageHWPoison if the read happens before the poison bit
>>>>>> is set and the write happens after. Will only get worse if/when
>>>>>> we add more non-atomic flag operations.
>>>>>>
>>>>>> Fix by acquiring zone->lock around TestSetPageHWPoison and
>>>>>> around ClearPageHWPoison in the retry path. This
>>>>>> serializes with all buddy flag manipulation. The cost is
>>>>>> negligible: one lock/unlock in an extremely rare path
>>>>>> (hardware memory errors).
>>>>>>
>>>>>> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
>>>>>> in this file operate on pages already removed from the buddy
>>>>>> allocator or on non-buddy pages (DAX, hugetlb), so they do not
>>>>>> need zone->lock protection.
>>>>>
>>>>> Sashiko is saying this doesn't do anything "Because
>>>>> __free_pages_prepare() executes entirely locklessly". Did it goof?
>>>>>
>>>>> https://sashiko.dev/#/patchset/df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com
>>>>
>>>> Battle of the bots: it's right.
>>>
>>> Yep, __free_pages_prepare() changes the page flag without holding
>>> zone->lock.
>>
>> __free_pages_prepare() works on frozen pages and assumes no one else
>> touches the input page. To avoid this race, memory_failure() might
>> want to try_get_page() before TestClearPageHWPoison(), but I am not
>> sure if that works along with memory failure flow.
>>
>> Best Regards,
>> Yan, Zi
>
>
>
> Actually memory failure already plays with this down the road no?
>
> So maybe it's enough to just SetPageHWPoison afterwards again?
>
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index ee42d4361309..4758fea94a96 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2415,6 +2415,7 @@ int memory_failure(unsigned long pfn, int flags)
> if (!res) {
> if (is_free_buddy_page(p)) {
> if (take_page_off_buddy(p)) {
> + SetPageHWPoison(p);
> page_ref_inc(p);
> res = MF_RECOVERED;
> } else {
>
>
> and maybe in a bunch of other places in there?
You mean for fear of losing HWPoison flag in the earlier TestSetPageHWPoison(),
just set it again here? Why not do it after get_hwpoison_page(), since that
is the expected page flag? Miaohe probably can give a better answer here.
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Michael S. Tsirkin @ 2026-06-09 20:34 UTC (permalink / raw)
To: Zi Yan
Cc: David Hildenbrand (Arm), Andrew Morton, linux-kernel, Miaohe Lin,
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: <38C84F23-E881-4DB2-86BA-93F39D44AE1B@nvidia.com>
On Tue, Jun 09, 2026 at 02:52:47PM -0400, Zi Yan wrote:
> On 9 Jun 2026, at 14:39, Zi Yan wrote:
>
> > On 9 Jun 2026, at 14:38, David Hildenbrand (Arm) wrote:
> >
> >> On 6/9/26 20:10, Andrew Morton wrote:
> >>> On Tue, 9 Jun 2026 06:12:49 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >>>
> >>>> TestSetPageHWPoison() is called without zone->lock, so its atomic
> >>>> update to page->flags can race with non-atomic flag operations
> >>>> that run under zone->lock in the buddy allocator.
> >>>>
> >>>> In particular, __free_pages_prepare() does:
> >>>>
> >>>> page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
> >>>>
> >>>> This non-atomic read-modify-write, while correctly excluding
> >>>> __PG_HWPOISON from the mask, can still lose a concurrent
> >>>> TestSetPageHWPoison if the read happens before the poison bit
> >>>> is set and the write happens after. Will only get worse if/when
> >>>> we add more non-atomic flag operations.
> >>>>
> >>>> Fix by acquiring zone->lock around TestSetPageHWPoison and
> >>>> around ClearPageHWPoison in the retry path. This
> >>>> serializes with all buddy flag manipulation. The cost is
> >>>> negligible: one lock/unlock in an extremely rare path
> >>>> (hardware memory errors).
> >>>>
> >>>> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
> >>>> in this file operate on pages already removed from the buddy
> >>>> allocator or on non-buddy pages (DAX, hugetlb), so they do not
> >>>> need zone->lock protection.
> >>>
> >>> Sashiko is saying this doesn't do anything "Because
> >>> __free_pages_prepare() executes entirely locklessly". Did it goof?
> >>>
> >>> https://sashiko.dev/#/patchset/df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com
> >>
> >> Battle of the bots: it's right.
> >
> > Yep, __free_pages_prepare() changes the page flag without holding
> > zone->lock.
>
> __free_pages_prepare() works on frozen pages and assumes no one else
> touches the input page. To avoid this race, memory_failure() might
> want to try_get_page() before TestClearPageHWPoison(), but I am not
> sure if that works along with memory failure flow.
>
> Best Regards,
> Yan, Zi
Actually memory failure already plays with this down the road no?
So maybe it's enough to just SetPageHWPoison afterwards again?
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index ee42d4361309..4758fea94a96 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2415,6 +2415,7 @@ int memory_failure(unsigned long pfn, int flags)
if (!res) {
if (is_free_buddy_page(p)) {
if (take_page_off_buddy(p)) {
+ SetPageHWPoison(p);
page_ref_inc(p);
res = MF_RECOVERED;
} else {
and maybe in a bunch of other places in there?
--
MST
^ permalink raw reply related
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Michael S. Tsirkin @ 2026-06-09 20:24 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, Miaohe Lin, David Hildenbrand (Arm), 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, 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
In-Reply-To: <20260609111020.e88f51a7b6ebc37360d66fdc@linux-foundation.org>
On Tue, Jun 09, 2026 at 11:10:20AM -0700, Andrew Morton wrote:
> On Tue, 9 Jun 2026 06:12:49 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > TestSetPageHWPoison() is called without zone->lock, so its atomic
> > update to page->flags can race with non-atomic flag operations
> > that run under zone->lock in the buddy allocator.
> >
> > In particular, __free_pages_prepare() does:
> >
> > page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
> >
> > This non-atomic read-modify-write, while correctly excluding
> > __PG_HWPOISON from the mask, can still lose a concurrent
> > TestSetPageHWPoison if the read happens before the poison bit
> > is set and the write happens after. Will only get worse if/when
> > we add more non-atomic flag operations.
> >
> > Fix by acquiring zone->lock around TestSetPageHWPoison and
> > around ClearPageHWPoison in the retry path. This
> > serializes with all buddy flag manipulation. The cost is
> > negligible: one lock/unlock in an extremely rare path
> > (hardware memory errors).
> >
> > Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
> > in this file operate on pages already removed from the buddy
> > allocator or on non-buddy pages (DAX, hugetlb), so they do not
> > need zone->lock protection.
>
> Sashiko is saying this doesn't do anything "Because
> __free_pages_prepare() executes entirely locklessly". Did it goof?
>
> https://sashiko.dev/#/patchset/df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com
>
Oh. So it only helps with the prezero patches. Maybe other places where
flags are touched locklessly. Not __free_pages_prepare. I was too
focused on that. Scrap this please. I'll try to think of something.
--
MST
^ permalink raw reply
* Re: [PATCH splitout] mm: page_reporting: allow driver to set batch capacity
From: Michael S. Tsirkin @ 2026-06-09 20:08 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, Miaohe Lin, David Hildenbrand (Arm), Jason Wang,
Xuan Zhuo, Eugenio Pérez, Muchun Song, Oscar Salvador,
Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi,
Alexander Duyck
In-Reply-To: <aihRBTaTUmgYmZfX@gourry-fedora-PF4VCD3F>
On Tue, Jun 09, 2026 at 01:44:37PM -0400, Gregory Price wrote:
> On Tue, Jun 09, 2026 at 11:53:20AM -0400, Michael S. Tsirkin wrote:
> > --- a/mm/page_reporting.c
> > +++ b/mm/page_reporting.c
> > @@ -174,10 +174,10 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
> > * 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.
> > + * The division here uses integer division; capacity need
> > + * not 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);
> >
>
> Initial look - is there a div-by-0 here? I noticed the old check
> prevents this from being (0 * 16), but i don't see (on first pass)
> the same check anywhere.
>
> Unless this line below always forces the above to be a
> PAGE_REPORTING_CAPCAITY if it's set to 0.
It does, does it not?
> > + if (!prdev->capacity || prdev->capacity > PAGE_REPORTING_CAPACITY)
> > + prdev->capacity = PAGE_REPORTING_CAPACITY;
> > +
>
> It's worth making this corner condition a little more obvious.
>
> The code intends for
>
> if (capacity == 0)
> capacity = PAGE_REPORTING_CAPACITY
>
> but that's not reflected in the changelog as a default value.
>
> When happens if a driver sets (capacity=0) either on purpose (???)
what would the purpose be? if you don't want reporting do not register.
> or
> because there's a bug (???)
exactly ??? since where are we practicing defensive programming in kernel
APIs?
> and then page_reporting.c forces it up to
> 32?
>
> There's something to improve here.
>
> ~Gregory
So I'll update the commit log to mention PAGE_REPORTING_CAPACITY.
And maybe a comment near capacity field?
Should be enough?
--
MST
^ permalink raw reply
* Re: [PATCH v4 02/47] x86/tsc: Add a standalone helpers for getting TSC info from CPUID.0x15
From: Sean Christopherson @ 2026-06-09 19:28 UTC (permalink / raw)
To: Borislav Petkov
Cc: Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
Kiryl Shutsemau, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov, Jan Kiszka,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
John Stultz, H. Peter Anvin, Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, kvm, linux-kernel, linux-coco, linux-hyperv,
virtualization, xen-devel, David Woodhouse, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, Michael Kelley,
Thomas Gleixner
In-Reply-To: <20260602034916.GGah5SvARd77mkvxe3@fat_crate.local>
On Mon, Jun 01, 2026, Borislav Petkov wrote:
> On Fri, May 29, 2026 at 07:43:49AM -0700, Sean Christopherson wrote:
> > +static int cpuid_get_tsc_info(struct cpuid_tsc_info *info)
> > +{
> > + unsigned int ecx_hz, edx;
> > +
> > + memset(info, 0, sizeof(*info));
>
> Let's not clear this unnecessarily...
>
> > +
> > + if (boot_cpu_data.cpuid_level < CPUID_LEAF_TSC)
> > + return -ENOENT;
>
> ... just to return here...
>
> > +
> > + /* CPUID 15H TSC/Crystal ratio, plus optionally Crystal Hz */
> > + cpuid(CPUID_LEAF_TSC, &info->denominator, &info->numerator, &ecx_hz, &edx);
> > +
> > + if (!info->denominator || !info->numerator)
> > + return -ENOENT;
>
> ... or here.
>
> We wanna clear it here, when we'll return success.
Actually, if we take the approach of relying on the user to check the return
code, then there's no need to zero the struct since all fields will be explicitly
written, especially if we drop the "tsc_khz" field. I was zeroing the field
purely as defense in depth.
^ permalink raw reply
* Re: [PATCH v4 01/47] x86/tsc: Never re-calibrate TSC frequency if its exact timing is known
From: Thomas Gleixner @ 2026-06-09 19:27 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov, Jan Kiszka,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
John Stultz, H. Peter Anvin, Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, kvm, linux-kernel, linux-coco, linux-hyperv,
virtualization, xen-devel, David Woodhouse, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, Michael Kelley
In-Reply-To: <aihKj-0nP7bUbNHH@google.com>
On Tue, Jun 09 2026 at 10:17, Sean Christopherson wrote:
> On Fri, Jun 05, 2026, Thomas Gleixner wrote:
>> On Fri, Jun 05 2026 at 11:04, Sean Christopherson wrote:
>> But we also should have a check in the TSC init code somewhere which
>> validates that X86_FEATURE_CONSTANT_TSC is set when
>> X86_FEATURE_TSC_KNOWN_FREQ is set. X86_FEATURE_TSC_KNOWN_FREQ is useless
>> w/o X86_FEATURE_CONSTANT_TSC.
>
> Ugh, any objection to punting on this for now? KVM and Xen guests will trigger
> TSC_KNOWN_FREQ without CONSTANT_TSC, thanks to commits:
>
> e10f78050323 ("kvmclock: fix TSC calibration for nested guests")
> 898ec52d2ba0 ("x86/xen/time: Set the X86_FEATURE_TSC_KNOWN_FREQ flag in xen_tsc_khz()")
>
> Hyper-V guests might as well? Hyper-V's handling of TSC is weird, even for a
> hypervisor.
Hypervisors are ranked by weirdness? I ranked them by insanity so far.
> Even when the frequency is provided in CPUID by the hypervisor, QEMU at least
> requires a fairly explicit opt-in to advertise CONSTANT_TSC, presumably to try
> to prevent users from shooting themselves in the foot.
Bah. We really should have enforced the dependency when we introduced
KNOWN_FREQ. But that ship has sailed.
Though for correctness sake this should be fixed at some point in the
foreseeable future.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Zi Yan @ 2026-06-09 18:52 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Andrew Morton, Michael S. Tsirkin, linux-kernel, Miaohe Lin,
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: <FB250C43-790E-4AB0-BD40-74665FC601A0@nvidia.com>
On 9 Jun 2026, at 14:39, Zi Yan wrote:
> On 9 Jun 2026, at 14:38, David Hildenbrand (Arm) wrote:
>
>> On 6/9/26 20:10, Andrew Morton wrote:
>>> On Tue, 9 Jun 2026 06:12:49 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>
>>>> TestSetPageHWPoison() is called without zone->lock, so its atomic
>>>> update to page->flags can race with non-atomic flag operations
>>>> that run under zone->lock in the buddy allocator.
>>>>
>>>> In particular, __free_pages_prepare() does:
>>>>
>>>> page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>>>>
>>>> This non-atomic read-modify-write, while correctly excluding
>>>> __PG_HWPOISON from the mask, can still lose a concurrent
>>>> TestSetPageHWPoison if the read happens before the poison bit
>>>> is set and the write happens after. Will only get worse if/when
>>>> we add more non-atomic flag operations.
>>>>
>>>> Fix by acquiring zone->lock around TestSetPageHWPoison and
>>>> around ClearPageHWPoison in the retry path. This
>>>> serializes with all buddy flag manipulation. The cost is
>>>> negligible: one lock/unlock in an extremely rare path
>>>> (hardware memory errors).
>>>>
>>>> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
>>>> in this file operate on pages already removed from the buddy
>>>> allocator or on non-buddy pages (DAX, hugetlb), so they do not
>>>> need zone->lock protection.
>>>
>>> Sashiko is saying this doesn't do anything "Because
>>> __free_pages_prepare() executes entirely locklessly". Did it goof?
>>>
>>> https://sashiko.dev/#/patchset/df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com
>>
>> Battle of the bots: it's right.
>
> Yep, __free_pages_prepare() changes the page flag without holding
> zone->lock.
__free_pages_prepare() works on frozen pages and assumes no one else
touches the input page. To avoid this race, memory_failure() might
want to try_get_page() before TestClearPageHWPoison(), but I am not
sure if that works along with memory failure flow.
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Zi Yan @ 2026-06-09 18:39 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Andrew Morton, Michael S. Tsirkin, linux-kernel, Miaohe Lin,
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: <8c1f468e-b50a-487a-a267-8d1ea5a61c87@kernel.org>
On 9 Jun 2026, at 14:38, David Hildenbrand (Arm) wrote:
> On 6/9/26 20:10, Andrew Morton wrote:
>> On Tue, 9 Jun 2026 06:12:49 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>
>>> TestSetPageHWPoison() is called without zone->lock, so its atomic
>>> update to page->flags can race with non-atomic flag operations
>>> that run under zone->lock in the buddy allocator.
>>>
>>> In particular, __free_pages_prepare() does:
>>>
>>> page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>>>
>>> This non-atomic read-modify-write, while correctly excluding
>>> __PG_HWPOISON from the mask, can still lose a concurrent
>>> TestSetPageHWPoison if the read happens before the poison bit
>>> is set and the write happens after. Will only get worse if/when
>>> we add more non-atomic flag operations.
>>>
>>> Fix by acquiring zone->lock around TestSetPageHWPoison and
>>> around ClearPageHWPoison in the retry path. This
>>> serializes with all buddy flag manipulation. The cost is
>>> negligible: one lock/unlock in an extremely rare path
>>> (hardware memory errors).
>>>
>>> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
>>> in this file operate on pages already removed from the buddy
>>> allocator or on non-buddy pages (DAX, hugetlb), so they do not
>>> need zone->lock protection.
>>
>> Sashiko is saying this doesn't do anything "Because
>> __free_pages_prepare() executes entirely locklessly". Did it goof?
>>
>> https://sashiko.dev/#/patchset/df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com
>
> Battle of the bots: it's right.
Yep, __free_pages_prepare() changes the page flag without holding
zone->lock.
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: David Hildenbrand (Arm) @ 2026-06-09 18:38 UTC (permalink / raw)
To: Andrew Morton, Michael S. Tsirkin
Cc: linux-kernel, Miaohe Lin, 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, 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
In-Reply-To: <20260609111020.e88f51a7b6ebc37360d66fdc@linux-foundation.org>
On 6/9/26 20:10, Andrew Morton wrote:
> On Tue, 9 Jun 2026 06:12:49 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>> TestSetPageHWPoison() is called without zone->lock, so its atomic
>> update to page->flags can race with non-atomic flag operations
>> that run under zone->lock in the buddy allocator.
>>
>> In particular, __free_pages_prepare() does:
>>
>> page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>>
>> This non-atomic read-modify-write, while correctly excluding
>> __PG_HWPOISON from the mask, can still lose a concurrent
>> TestSetPageHWPoison if the read happens before the poison bit
>> is set and the write happens after. Will only get worse if/when
>> we add more non-atomic flag operations.
>>
>> Fix by acquiring zone->lock around TestSetPageHWPoison and
>> around ClearPageHWPoison in the retry path. This
>> serializes with all buddy flag manipulation. The cost is
>> negligible: one lock/unlock in an extremely rare path
>> (hardware memory errors).
>>
>> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
>> in this file operate on pages already removed from the buddy
>> allocator or on non-buddy pages (DAX, hugetlb), so they do not
>> need zone->lock protection.
>
> Sashiko is saying this doesn't do anything "Because
> __free_pages_prepare() executes entirely locklessly". Did it goof?
>
> https://sashiko.dev/#/patchset/df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com
Battle of the bots: it's right.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Andrew Morton @ 2026-06-09 18:10 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Miaohe Lin, David Hildenbrand (Arm), 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, 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
In-Reply-To: <df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com>
On Tue, 9 Jun 2026 06:12:49 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> TestSetPageHWPoison() is called without zone->lock, so its atomic
> update to page->flags can race with non-atomic flag operations
> that run under zone->lock in the buddy allocator.
>
> In particular, __free_pages_prepare() does:
>
> page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>
> This non-atomic read-modify-write, while correctly excluding
> __PG_HWPOISON from the mask, can still lose a concurrent
> TestSetPageHWPoison if the read happens before the poison bit
> is set and the write happens after. Will only get worse if/when
> we add more non-atomic flag operations.
>
> Fix by acquiring zone->lock around TestSetPageHWPoison and
> around ClearPageHWPoison in the retry path. This
> serializes with all buddy flag manipulation. The cost is
> negligible: one lock/unlock in an extremely rare path
> (hardware memory errors).
>
> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
> in this file operate on pages already removed from the buddy
> allocator or on non-buddy pages (DAX, hugetlb), so they do not
> need zone->lock protection.
Sashiko is saying this doesn't do anything "Because
__free_pages_prepare() executes entirely locklessly". Did it goof?
https://sashiko.dev/#/patchset/df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com
^ permalink raw reply
* [PATCH] drm/virtio: Use common error handling code in two functions
From: Markus Elfring @ 2026-06-09 18:08 UTC (permalink / raw)
To: virtualization, dri-devel, Chia-I Wu, Dmitry Osipenko,
David Airlie, Gerd Hoffmann, Gurchetan Singh, Maarten Lankhorst,
Maxime Ripard, Simona Vetter, Thomas Zimmermann
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 Jun 2026 20:00:07 +0200
Use additional labels so that a bit of exception handling can be better
reused at the end of two function implementations.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 7 +++----
drivers/gpu/drm/virtio/virtgpu_vram.c | 16 ++++++++--------
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 67865810a2e7..05b19c73103a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -318,15 +318,14 @@ static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size, int *sg_ents)
*sg_ents = DIV_ROUND_UP(size, PAGE_SIZE);
ret = sg_alloc_table(sgt, *sg_ents, GFP_KERNEL);
- if (ret) {
- kfree(sgt);
- return NULL;
- }
+ if (ret)
+ goto free_sgt;
for_each_sgtable_sg(sgt, sg, i) {
pg = vmalloc_to_page(data);
if (!pg) {
sg_free_table(sgt);
+free_sgt:
kfree(sgt);
return NULL;
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_vram.c b/drivers/gpu/drm/virtio/virtgpu_vram.c
index 4ae3cbc35dd3..ec5b669fccfa 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vram.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vram.c
@@ -212,16 +212,12 @@ int virtio_gpu_vram_create(struct virtio_gpu_device *vgdev,
/* Create fake offset */
ret = drm_gem_create_mmap_offset(obj);
- if (ret) {
- kfree(vram);
- return ret;
- }
+ if (ret)
+ goto free_vram;
ret = virtio_gpu_resource_id_get(vgdev, &vram->base.hw_res_handle);
- if (ret) {
- kfree(vram);
- return ret;
- }
+ if (ret)
+ goto free_vram;
virtio_gpu_cmd_resource_create_blob(vgdev, &vram->base, params, NULL,
0);
@@ -237,6 +233,10 @@ int virtio_gpu_vram_create(struct virtio_gpu_device *vgdev,
*bo_ptr = &vram->base;
return 0;
+
+free_vram:
+ kfree(vram);
+ return ret;
}
void virtio_gpu_vram_map_deferred(struct virtio_gpu_object_vram *vram)
--
2.54.0
^ permalink raw reply related
* Re: [PATCH net] vsock/virtio: restore msg_iter on transmission failure
From: Octavian Purdila @ 2026-06-09 17:58 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, syzbot+28e5f3d207b14bae122a, Stefan Hajnoczi,
Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Arseniy Krasnov, kvm, virtualization, linux-kernel
In-Reply-To: <aifL0f_QO1kocccU@sgarzare-redhat>
On Tue, Jun 9, 2026 at 1:48 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Tue, Jun 09, 2026 at 12:48:05AM +0000, Octavian Purdila wrote:
> >When transmission fails in virtio_transport_send_pkt_info, the msg_iter
> >might have been partially advanced. If we don't restore it, the next
> >attempt to send data will use an incorrect iterator state, leading to
> >desync and warnings like "send_pkt() returns 0, but X expected".
>
> Thanks for the fix! I have some comments.
Thank you for the quick review!
> >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> >index b10666937c490..588623a3e2bbc 100644
> >--- a/net/vmw_vsock/virtio_transport_common.c
> >+++ b/net/vmw_vsock/virtio_transport_common.c
> >@@ -367,6 +367,10 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> > do {
> > struct sk_buff *skb;
> > size_t skb_len;
> >+ struct iov_iter saved_iter;
>
> trivial: reverse xmas tree:
> https://docs.kernel.org/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs
>
I'll fix it in v2, sorry for missing this.
> >+
> >+ if (info->msg)
> >+ saved_iter = info->msg->msg_iter;
>
> What about using iov_iter_save_state()/iov_iter_restore() ?
>
> IIUC we may need to export iov_iter_restore(), so not a strong opinion,
> but it looks better to use those API IMHO.
>
I agree, I'll add the export as a separate patch in v2 and move to
using these APIs.
> >
> > skb_len = min(max_skb_len, rest_len);
> >
> >@@ -375,6 +379,8 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> > src_cid, src_port,
> > dst_cid, dst_port);
>
> What about adding a comment on top of virtio_transport_alloc_skb() call
> (or when we save the state) to explain that in specific cases it can
> advance the msg_iter ?
>
Good point, I will add a comment explaining this in v2.
> > if (!skb) {
> >+ if (info->msg)
> >+ info->msg->msg_iter = saved_iter;
> > ret = -ENOMEM;
> > break;
> > }
> >@@ -382,8 +388,11 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> > virtio_transport_inc_tx_pkt(vvs, skb);
> >
> > ret = t_ops->send_pkt(skb, info->net);
> >- if (ret < 0)
> >+ if (ret < 0) {
> >+ if (info->msg)
> >+ info->msg->msg_iter = saved_iter;
>
> Also, what about having a single restore point after the loop?
>
> I mean something like this (untested):
>
Yes, that looks much better. I tested it locally and it works fine. Thanks!
I'll follow-up with v2 in a day or two.
^ permalink raw reply
* Re: [PATCH splitout] mm: page_reporting: allow driver to set batch capacity
From: Gregory Price @ 2026-06-09 17:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Miaohe Lin, David Hildenbrand (Arm), Jason Wang,
Xuan Zhuo, Eugenio Pérez, Muchun Song, Oscar Salvador,
Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Naoya Horiguchi,
Alexander Duyck
In-Reply-To: <b25ea6c63503f24c8b5b64910a44dc03b5aa610d.1781020302.git.mst@redhat.com>
On Tue, Jun 09, 2026 at 11:53:20AM -0400, Michael S. Tsirkin wrote:
> --- a/mm/page_reporting.c
> +++ b/mm/page_reporting.c
> @@ -174,10 +174,10 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
> * 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.
> + * The division here uses integer division; capacity need
> + * not 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);
>
Initial look - is there a div-by-0 here? I noticed the old check
prevents this from being (0 * 16), but i don't see (on first pass)
the same check anywhere.
Unless this line below always forces the above to be a
PAGE_REPORTING_CAPCAITY if it's set to 0.
> + if (!prdev->capacity || prdev->capacity > PAGE_REPORTING_CAPACITY)
> + prdev->capacity = PAGE_REPORTING_CAPACITY;
> +
It's worth making this corner condition a little more obvious.
The code intends for
if (capacity == 0)
capacity = PAGE_REPORTING_CAPACITY
but that's not reflected in the changelog as a default value.
When happens if a driver sets (capacity=0) either on purpose (???) or
because there's a bug (???) and then page_reporting.c forces it up to
32?
There's something to improve here.
~Gregory
^ permalink raw reply
* Re: [PATCH v4 01/47] x86/tsc: Never re-calibrate TSC frequency if its exact timing is known
From: Sean Christopherson @ 2026-06-09 17:17 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Paolo Bonzini, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov, Jan Kiszka,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
John Stultz, H. Peter Anvin, Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, kvm, linux-kernel, linux-coco, linux-hyperv,
virtualization, xen-devel, David Woodhouse, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, Michael Kelley
In-Reply-To: <87a4t86a0l.ffs@fw13>
On Fri, Jun 05, 2026, Thomas Gleixner wrote:
> On Fri, Jun 05 2026 at 11:04, Sean Christopherson wrote:
> But we also should have a check in the TSC init code somewhere which
> validates that X86_FEATURE_CONSTANT_TSC is set when
> X86_FEATURE_TSC_KNOWN_FREQ is set. X86_FEATURE_TSC_KNOWN_FREQ is useless
> w/o X86_FEATURE_CONSTANT_TSC.
Ugh, any objection to punting on this for now? KVM and Xen guests will trigger
TSC_KNOWN_FREQ without CONSTANT_TSC, thanks to commits:
e10f78050323 ("kvmclock: fix TSC calibration for nested guests")
898ec52d2ba0 ("x86/xen/time: Set the X86_FEATURE_TSC_KNOWN_FREQ flag in xen_tsc_khz()")
Hyper-V guests might as well? Hyper-V's handling of TSC is weird, even for a
hypervisor.
Even when the frequency is provided in CPUID by the hypervisor, QEMU at least
requires a fairly explicit opt-in to advertise CONSTANT_TSC, presumably to try
to prevent users from shooting themselves in the foot.
^ permalink raw reply
* [PATCH splitout] virtio_balloon: disable indirect descriptors
From: Michael S. Tsirkin @ 2026-06-09 16:33 UTC (permalink / raw)
To: linux-kernel
Cc: Miaohe Lin, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, 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
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.
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);
__virtio_clear_bit(vdev, VIRTIO_F_ACCESS_PLATFORM);
return 0;
}
--
MST
^ permalink raw reply related
* Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock
From: Zi Yan @ 2026-06-09 16:12 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Miaohe Lin, David Hildenbrand (Arm), Jason Wang,
Xuan Zhuo, Eugenio Pérez, Muchun Song, Oscar Salvador,
Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, 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: <df06b66fe4ff8e925ee0714955abc2183a727b90.1780998980.git.mst@redhat.com>
On 9 Jun 2026, at 6:12, Michael S. Tsirkin wrote:
> TestSetPageHWPoison() is called without zone->lock, so its atomic
> update to page->flags can race with non-atomic flag operations
> that run under zone->lock in the buddy allocator.
>
> In particular, __free_pages_prepare() does:
>
> page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>
> This non-atomic read-modify-write, while correctly excluding
> __PG_HWPOISON from the mask, can still lose a concurrent
> TestSetPageHWPoison if the read happens before the poison bit
> is set and the write happens after. Will only get worse if/when
> we add more non-atomic flag operations.
>
> Fix by acquiring zone->lock around TestSetPageHWPoison and
> around ClearPageHWPoison in the retry path. This
> serializes with all buddy flag manipulation. The cost is
> negligible: one lock/unlock in an extremely rare path
> (hardware memory errors).
>
> Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
> in this file operate on pages already removed from the buddy
> allocator or on non-buddy pages (DAX, hugetlb), so they do not
> need zone->lock protection.
>
> Fixes: 6a46079cf57a ("HWPOISON: The high level memory error handler in the VM v7")
> Acked-by: Miaohe Lin <linmiaohe@huawei.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---
>
> Sending separately as suggested by multiple people. I also added
> a Fixes tag.
>
>
> mm/memory-failure.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
Makes sense to me. Thanks.
Acked-by: Zi Yan <ziy@nvidia.com>
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH v2] virtio-blk: clamp zone report to the report buffer capacity
From: Jens Axboe @ 2026-06-09 16:02 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, Stefan Hajnoczi,
Michael Bommarito
Cc: Xuan Zhuo, virtualization, linux-block, linux-kernel
In-Reply-To: <20260607124834.3059944-1-michael.bommarito@gmail.com>
On Sun, 07 Jun 2026 08:48:34 -0400, Michael Bommarito wrote:
> virtblk_report_zones() trusts the device-reported number of zones when
> walking the report buffer:
>
> nz = min_t(u64, virtio64_to_cpu(vblk->vdev, report->nr_zones),
> nr_zones);
> ...
> for (i = 0; i < nz && zone_idx < nr_zones; i++) {
> ret = virtblk_parse_zone(vblk, &report->zones[i], ...);
>
> [...]
Applied, thanks!
[1/1] virtio-blk: clamp zone report to the report buffer capacity
commit: 0fd835f5e9477ebea2439b8ada58f34e1b8cf25a
Best regards,
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH v2] virtio-blk: clamp zone report to the report buffer capacity
From: Michael S. Tsirkin @ 2026-06-09 15:54 UTC (permalink / raw)
To: Michael Bommarito
Cc: Jason Wang, Stefan Hajnoczi, Jens Axboe, Xuan Zhuo,
virtualization, linux-block, linux-kernel
In-Reply-To: <20260607124834.3059944-1-michael.bommarito@gmail.com>
On Sun, Jun 07, 2026 at 08:48:34AM -0400, Michael Bommarito wrote:
> virtblk_report_zones() trusts the device-reported number of zones when
> walking the report buffer:
>
> nz = min_t(u64, virtio64_to_cpu(vblk->vdev, report->nr_zones),
> nr_zones);
> ...
> for (i = 0; i < nz && zone_idx < nr_zones; i++) {
> ret = virtblk_parse_zone(vblk, &report->zones[i], ...);
>
> The buffer is allocated by virtblk_alloc_report_buffer(), whose size is
> capped by the queue's max hardware sectors and max segments and can
> therefore hold fewer descriptors than nr_zones. nz is bounded only by
> the device-supplied report->nr_zones and the requested nr_zones, never
> by the buffer's descriptor capacity. At probe time the request count is
> unbounded (blk_revalidate_disk_zones() calls report_zones() with
> nr_zones == UINT_MAX), so the device-supplied report->nr_zones is the
> sole gate: a device that reports more zones than fit in the buffer
> drives the loop to read report->zones[i] past the end of the allocation.
>
> A malicious or buggy virtio-blk device that reports an inflated nr_zones
> triggers this during zone revalidation at probe. KASAN reports a
> vmalloc-out-of-bounds read in virtblk_report_zones() against the report
> buffer allocated a few lines earlier.
>
> Clamp nz to the number of descriptors that actually fit in the report
> buffer.
>
> Fixes: 95bfec41bd3d ("virtio-blk: add support for zoned block devices")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> v2: drop the explanatory comment per Michael S. Tsirkin's review; the
> clamp itself is unchanged.
>
> drivers/block/virtio_blk.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index b1c9a27..32bf3ba 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -689,6 +689,8 @@ static int virtblk_report_zones(struct gendisk *disk, sector_t sector,
>
> nz = min_t(u64, virtio64_to_cpu(vblk->vdev, report->nr_zones),
> nr_zones);
> + nz = min_t(u64, nz,
> + (buflen - sizeof(*report)) / sizeof(report->zones[0]));
> if (!nz)
> break;
>
>
> base-commit: 5200f5f493f79f14bbdc349e402a40dfb32f23c8
> --
> 2.53.0
^ permalink raw reply
* [PATCH splitout] mm: page_reporting: allow driver to set batch capacity
From: Michael S. Tsirkin @ 2026-06-09 15:53 UTC (permalink / raw)
To: linux-kernel
Cc: Miaohe Lin, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, 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
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.
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 | 5 +----
include/linux/page_reporting.h | 3 +++
mm/page_reporting.c | 25 ++++++++++++++-----------
3 files changed, 18 insertions(+), 15 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..5ab5be02fa15 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 (default 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..5b6b17f67131 100644
--- a/mm/page_reporting.c
+++ b/mm/page_reporting.c
@@ -174,10 +174,10 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
* 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.
+ * The division here uses integer division; capacity need
+ * not 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 +222,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 +234,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 +260,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 +290,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 +322,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 +377,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);
--
MST
^ permalink raw reply related
* Re: [PATCH v2] virtio-blk: clamp zone report to the report buffer capacity
From: Stefan Hajnoczi @ 2026-06-09 15:09 UTC (permalink / raw)
To: Michael Bommarito
Cc: Michael S . Tsirkin, Jason Wang, Jens Axboe, Xuan Zhuo,
virtualization, linux-block, linux-kernel
In-Reply-To: <20260607124834.3059944-1-michael.bommarito@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]
On Sun, Jun 07, 2026 at 08:48:34AM -0400, Michael Bommarito wrote:
> virtblk_report_zones() trusts the device-reported number of zones when
> walking the report buffer:
>
> nz = min_t(u64, virtio64_to_cpu(vblk->vdev, report->nr_zones),
> nr_zones);
> ...
> for (i = 0; i < nz && zone_idx < nr_zones; i++) {
> ret = virtblk_parse_zone(vblk, &report->zones[i], ...);
>
> The buffer is allocated by virtblk_alloc_report_buffer(), whose size is
> capped by the queue's max hardware sectors and max segments and can
> therefore hold fewer descriptors than nr_zones. nz is bounded only by
> the device-supplied report->nr_zones and the requested nr_zones, never
> by the buffer's descriptor capacity. At probe time the request count is
> unbounded (blk_revalidate_disk_zones() calls report_zones() with
> nr_zones == UINT_MAX), so the device-supplied report->nr_zones is the
> sole gate: a device that reports more zones than fit in the buffer
> drives the loop to read report->zones[i] past the end of the allocation.
>
> A malicious or buggy virtio-blk device that reports an inflated nr_zones
> triggers this during zone revalidation at probe. KASAN reports a
> vmalloc-out-of-bounds read in virtblk_report_zones() against the report
> buffer allocated a few lines earlier.
>
> Clamp nz to the number of descriptors that actually fit in the report
> buffer.
>
> Fixes: 95bfec41bd3d ("virtio-blk: add support for zoned block devices")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> v2: drop the explanatory comment per Michael S. Tsirkin's review; the
> clamp itself is unchanged.
>
> drivers/block/virtio_blk.c | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net] virtio_net: do not allow tunnel csum offload for non GSO packets
From: Michael S. Tsirkin @ 2026-06-09 14:50 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Jason Wang, Xuan Zhuo, Eugenio Pérez, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, virtualization,
Willem de Bruijn
In-Reply-To: <38d33d63ce5d1aee486bd6c7c47345040d526e35.1781016169.git.pabeni@redhat.com>
On Tue, Jun 09, 2026 at 04:44:26PM +0200, Paolo Abeni wrote:
> Fiona reports broken connectivity for virtio net setup using UDP tunnel
> inside the guest and NIC with not UDP tunnel TSO support in the host.
>
> Currently the virtio_net driver exposes csum offload for UDP-tunneled,
> TCP non GSO packets. Such packet reach the host as CSUM_PARTIAL ones
> with the 'encapsulation' flag cleared, as the virtio specification do
> not support this specific kind of offload.
>
> HW NICs with UDP tunnel TSO support - and those drivers directly
> accessing skb->csum_start/csum_offset - are still capable of computing
> the needed csum correctly, but otherwise the packets reach the wire with
> bad csum on both the inner and outer transport header.
>
> Address the issue explicitly disabling csum offload for UDP tunneled,
> non GSO packets via the ndo_features_check op.
>
> Fixes: 56a06bd40fab ("virtio_net: enable gso over UDP tunnel support.")
> Reported-by: Fiona Ebner <f.ebner@proxmox.com>
> Closes: https://bugzilla.proxmox.com/show_bug.cgi?id=7627
> Tested-by: Fiona Ebner <f.ebner@proxmox.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f4adcfee7a80..07b8710639f9 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -6222,6 +6222,18 @@ static void virtnet_free_irq_moder(struct virtnet_info *vi)
> rtnl_unlock();
> }
>
> +static netdev_features_t virtnet_features_check(struct sk_buff *skb,
> + struct net_device *dev,
> + netdev_features_t features)
> +{
> + /* Inner csum offload is only available for GSO packets. */
> + if (skb->encapsulation && !skb_is_gso(skb))
> + return features & ~NETIF_F_CSUM_MASK;
> +
> + /* Passthru. */
> + return features;
> +}
> +
> static const struct net_device_ops virtnet_netdev = {
> .ndo_open = virtnet_open,
> .ndo_stop = virtnet_close,
> @@ -6235,7 +6247,7 @@ static const struct net_device_ops virtnet_netdev = {
> .ndo_bpf = virtnet_xdp,
> .ndo_xdp_xmit = virtnet_xdp_xmit,
> .ndo_xsk_wakeup = virtnet_xsk_wakeup,
> - .ndo_features_check = passthru_features_check,
> + .ndo_features_check = virtnet_features_check,
> .ndo_get_phys_port_name = virtnet_get_phys_port_name,
> .ndo_set_features = virtnet_set_features,
> .ndo_tx_timeout = virtnet_tx_timeout,
> --
> 2.54.0
^ 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