* Re: [PATCH resend v6 10/30] mm: alloc_swap_folio: pass raw fault address to vma_alloc_folio
From: Michael S. Tsirkin @ 2026-05-11 22:02 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett
In-Reply-To: <agJPpxyu4gKqq8rj@gourry-fedora-PF4VCD3F>
On Mon, May 11, 2026 at 05:52:39PM -0400, Gregory Price wrote:
> On Mon, May 11, 2026 at 05:41:16PM -0400, Michael S. Tsirkin wrote:
> > On Mon, May 11, 2026 at 12:05:18PM -0400, Gregory Price wrote:
> > > On Mon, May 11, 2026 at 05:02:39AM -0400, Michael S. Tsirkin wrote:
> > > > Same change as the previous patch but for alloc_swap_folio:
> > > > pass vmf->address directly instead of ALIGN_DOWN(vmf->address, ...).
> > > >
> > >
> > > Starting to seem like this should all just get squashed into once patch.
> > >
> > > ~Gregory
> >
> > I mean I was told the patches are too big and too hard to review.
> > It's just same, independent changes made in several places.
> > Does it matter if they are squashed or not?
> >
>
> Mostly i think the ordering of the patches thrashes from one set
> (alignment) to another set (USER_ADDR_NONE). If all of one set was
> pulled ahead of the other then it would be easier to follow.
>
> This particular set seemed trivial enough to just be one patch, but i
> don't think it matters all that much.
>
> ~Gregory
I can reorder and put alignment before threading, sure, that's trivial:
these patches are all independent and cause no conflicts when
reordering.
So far the main work will be the hugetlb thing - it really parallels
what I did with post_alloc_hook, to think of it.
Will take me a couple of days, I'm behind on other tasks.
--
MST
^ permalink raw reply
* Re: [PATCH resend v6 03/30] mm: thread user_addr through page allocator for cache-friendly zeroing
From: Michael S. Tsirkin @ 2026-05-11 21:59 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett,
Harry Yoo, Hao Li
In-Reply-To: <agIJOkLhTOsvJs1-@gourry-fedora-PF4VCD3F>
On Mon, May 11, 2026 at 12:52:10PM -0400, Gregory Price wrote:
> On Mon, May 11, 2026 at 11:55:40AM -0400, Michael S. Tsirkin wrote:
> > On Mon, May 11, 2026 at 11:37:37AM -0400, Gregory Price wrote:
> > > On Mon, May 11, 2026 at 05:01:55AM -0400, Michael S. Tsirkin wrote:
> > >
> > > > +/*
> > > > + * Sentinel for user_addr: indicates a non-user allocation.
> > > > + * Cannot use 0 because address 0 is a valid userspace mapping.
> > > > + */
> > > > +#define USER_ADDR_NONE ((unsigned long)-1)
> > >
> > > Ehm, hm. Does -1 hold as a non-user address across all architectures?
> > >
> > > What about in linear addressing / no VM mode?
> >
> > this is used on a fault. I don't think there are any faults then?
> > But maybe FAULT_ADDR_NONE would be clearer.
> >
>
> Meh, naming here is less relevant than the sentinel correctness.
>
> My only concern is really whether -1 could end up being a valid address
> in some horrid future timeline and this all going belly up.
>
> Is why I asked about whether this is correct on all architectures.
I think the answer is yes: on all architectures Linux supports, the last
page of the address space is never a valid user mapping. The kernel
enforces this -- mmap will not create a mapping whose end wraps past -1.
if (addr > TASK_SIZE - len)
return -ENOMEM;
So the maximum vm_end = addr + len <= TASK_SIZE.
On every architecture TASK_SIZE <= (unsigned long)-1, so vm_end can
never wrap around to 0 and address -1 can never be within any
VMA.
> > > So the trade off is:
> > > a) churn the current interface for everyone
> > > b) add a user_ variant and know people will just get it wrong
> >
> > I was also explicitly asked not to proliferate too many new APIs.
> >
>
> Yeah simply spelling it out, not asking for a change. Probably no
> better way to go about it.
>
> ~Gregory
^ permalink raw reply
* Re: [PATCH resend v6 10/30] mm: alloc_swap_folio: pass raw fault address to vma_alloc_folio
From: Gregory Price @ 2026-05-11 21:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett
In-Reply-To: <20260511174032-mutt-send-email-mst@kernel.org>
On Mon, May 11, 2026 at 05:41:16PM -0400, Michael S. Tsirkin wrote:
> On Mon, May 11, 2026 at 12:05:18PM -0400, Gregory Price wrote:
> > On Mon, May 11, 2026 at 05:02:39AM -0400, Michael S. Tsirkin wrote:
> > > Same change as the previous patch but for alloc_swap_folio:
> > > pass vmf->address directly instead of ALIGN_DOWN(vmf->address, ...).
> > >
> >
> > Starting to seem like this should all just get squashed into once patch.
> >
> > ~Gregory
>
> I mean I was told the patches are too big and too hard to review.
> It's just same, independent changes made in several places.
> Does it matter if they are squashed or not?
>
Mostly i think the ordering of the patches thrashes from one set
(alignment) to another set (USER_ADDR_NONE). If all of one set was
pulled ahead of the other then it would be easier to follow.
This particular set seemed trivial enough to just be one patch, but i
don't think it matters all that much.
~Gregory
^ permalink raw reply
* Re: [PATCH resend v6 14/30] mm: hugetlb: use __GFP_ZERO and skip zeroing for zeroed pages
From: Michael S. Tsirkin @ 2026-05-11 21:47 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <agIFq63QZugkWcXJ@gourry-fedora-PF4VCD3F>
On Mon, May 11, 2026 at 12:36:59PM -0400, Gregory Price wrote:
> On Mon, May 11, 2026 at 05:03:02AM -0400, Michael S. Tsirkin wrote:
> > Convert the hugetlb fault and fallocate paths to use __GFP_ZERO.
> > For pages allocated from the buddy allocator, post_alloc_hook()
> > handles zeroing.
> >
> > Hugetlb surplus pages need special handling because they can be
> > pre-allocated into the pool during mmap (by hugetlb_acct_memory)
> > before any page fault. Pool pages are kept around and may need
> > zeroing long after buddy allocation, so a buddy-level zeroed
> > hint (consumed at allocation time) cannot track their state.
> >
> > Add a bool *zeroed output parameter to alloc_hugetlb_folio()
> > so callers know whether the page needs zeroing. Buddy-allocated
> > pages are always zeroed (zeroed by post_alloc_hook). Pool
> > pages use a new HPG_zeroed flag to track whether the page is
> > known-zero (freshly buddy-allocated, never mapped to userspace).
> > The flag is set in alloc_surplus_hugetlb_folio() after buddy
> > allocation and cleared in free_huge_folio() when a user-mapped
> > page returns to the pool.
> >
> > Callers that do not need zeroing (CoW, migration) pass NULL for
> > zeroed and 0 for gfp.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Assisted-by: Claude:claude-opus-4-6
> > Assisted-by: cursor-agent:GPT-5.4-xhigh
> > ---
> > fs/hugetlbfs/inode.c | 10 ++++++--
> > include/linux/hugetlb.h | 8 +++++--
> > mm/hugetlb.c | 52 ++++++++++++++++++++++++++++++-----------
> > 3 files changed, 53 insertions(+), 17 deletions(-)
> >
> > diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> > index 8b05bec08e04..24e42cb10ade 100644
> > --- a/fs/hugetlbfs/inode.c
> > +++ b/fs/hugetlbfs/inode.c
> > @@ -810,14 +810,20 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
> > * folios in these areas, we need to consume the reserves
> > * to keep reservation accounting consistent.
> > */
> > - folio = alloc_hugetlb_folio(&pseudo_vma, addr, false);
> > + {
> > + bool zeroed;
> > +
> > + folio = alloc_hugetlb_folio(&pseudo_vma, addr, false,
> > + __GFP_ZERO, &zeroed);
>
> This feels like a very odd pattern:
>
> 1) ask for __GFP_ZERO
> 2) Have to check whether it was actually zeroed
>
> Seems like the zeroing piece should just be sunk in if you're going to
> ask for __GFP_ZERO anyway. And in that case, maybe just `bool zero` as
> an argument, rather than GFP (to avoid future overloading of flags).
>
> ~Gregory
Heh. The reason is that it either allocates from buddy - using gfp flags
or from the pool, in which case it zeroes.
We could even avoid the bool - just test __GFP_ZERO inside
alloc_hugetlb_folio. Would that be better?
--
MST
^ permalink raw reply
* Re: [PATCH resend v6 10/30] mm: alloc_swap_folio: pass raw fault address to vma_alloc_folio
From: Michael S. Tsirkin @ 2026-05-11 21:41 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett
In-Reply-To: <agH-PrpY269pyuJQ@gourry-fedora-PF4VCD3F>
On Mon, May 11, 2026 at 12:05:18PM -0400, Gregory Price wrote:
> On Mon, May 11, 2026 at 05:02:39AM -0400, Michael S. Tsirkin wrote:
> > Same change as the previous patch but for alloc_swap_folio:
> > pass vmf->address directly instead of ALIGN_DOWN(vmf->address, ...).
> >
>
> Starting to seem like this should all just get squashed into once patch.
>
> ~Gregory
I mean I was told the patches are too big and too hard to review.
It's just same, independent changes made in several places.
Does it matter if they are squashed or not?
^ permalink raw reply
* Re: [PATCH resend v6 03/30] mm: thread user_addr through page allocator for cache-friendly zeroing
From: Gregory Price @ 2026-05-11 16:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett,
Harry Yoo, Hao Li
In-Reply-To: <20260511114853-mutt-send-email-mst@kernel.org>
On Mon, May 11, 2026 at 11:55:40AM -0400, Michael S. Tsirkin wrote:
> On Mon, May 11, 2026 at 11:37:37AM -0400, Gregory Price wrote:
> > On Mon, May 11, 2026 at 05:01:55AM -0400, Michael S. Tsirkin wrote:
> >
> > > +/*
> > > + * Sentinel for user_addr: indicates a non-user allocation.
> > > + * Cannot use 0 because address 0 is a valid userspace mapping.
> > > + */
> > > +#define USER_ADDR_NONE ((unsigned long)-1)
> >
> > Ehm, hm. Does -1 hold as a non-user address across all architectures?
> >
> > What about in linear addressing / no VM mode?
>
> this is used on a fault. I don't think there are any faults then?
> But maybe FAULT_ADDR_NONE would be clearer.
>
Meh, naming here is less relevant than the sentinel correctness.
My only concern is really whether -1 could end up being a valid address
in some horrid future timeline and this all going belly up.
Is why I asked about whether this is correct on all architectures.
> > So the trade off is:
> > a) churn the current interface for everyone
> > b) add a user_ variant and know people will just get it wrong
>
> I was also explicitly asked not to proliferate too many new APIs.
>
Yeah simply spelling it out, not asking for a change. Probably no
better way to go about it.
~Gregory
^ permalink raw reply
* Re: [PATCH resend v6 15/30] mm: memfd: skip zeroing for zeroed hugetlb pool pages
From: Gregory Price @ 2026-05-11 16:39 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <a39ab8288aceb244bdb31141aa9cf67106c0e042.1778489843.git.mst@redhat.com>
On Mon, May 11, 2026 at 05:03:08AM -0400, Michael S. Tsirkin wrote:
> gather_surplus_pages() pre-allocates hugetlb pages into the pool
> during mmap. Pass __GFP_ZERO so these pages are zeroed by the
> buddy allocator, and HPG_zeroed is set by alloc_surplus_hugetlb_folio.
>
> Add bool *zeroed output to alloc_hugetlb_folio_reserve() so
> callers can check whether the pool page is known-zero. memfd's
> memfd_alloc_folio() uses this to skip the explicit folio_zero_user()
> when the page is already zero.
>
> This avoids redundant zeroing for memfd hugetlb pages that were
> pre-allocated into the pool and never mapped to userspace.
>
Same thing here, seems more like what's actually wanted is to sink the
folio_user_zero stuff into hugetlb.c rather than make memfd.c request
for zeroing - and then have to check again anyway.
~Gregory
^ permalink raw reply
* Re: [PATCH resend v6 14/30] mm: hugetlb: use __GFP_ZERO and skip zeroing for zeroed pages
From: Gregory Price @ 2026-05-11 16:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <fe1d24d4deafe0f5d1a9c51fcc040d9561c8592a.1778489843.git.mst@redhat.com>
On Mon, May 11, 2026 at 05:03:02AM -0400, Michael S. Tsirkin wrote:
> Convert the hugetlb fault and fallocate paths to use __GFP_ZERO.
> For pages allocated from the buddy allocator, post_alloc_hook()
> handles zeroing.
>
> Hugetlb surplus pages need special handling because they can be
> pre-allocated into the pool during mmap (by hugetlb_acct_memory)
> before any page fault. Pool pages are kept around and may need
> zeroing long after buddy allocation, so a buddy-level zeroed
> hint (consumed at allocation time) cannot track their state.
>
> Add a bool *zeroed output parameter to alloc_hugetlb_folio()
> so callers know whether the page needs zeroing. Buddy-allocated
> pages are always zeroed (zeroed by post_alloc_hook). Pool
> pages use a new HPG_zeroed flag to track whether the page is
> known-zero (freshly buddy-allocated, never mapped to userspace).
> The flag is set in alloc_surplus_hugetlb_folio() after buddy
> allocation and cleared in free_huge_folio() when a user-mapped
> page returns to the pool.
>
> Callers that do not need zeroing (CoW, migration) pass NULL for
> zeroed and 0 for gfp.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh
> ---
> fs/hugetlbfs/inode.c | 10 ++++++--
> include/linux/hugetlb.h | 8 +++++--
> mm/hugetlb.c | 52 ++++++++++++++++++++++++++++++-----------
> 3 files changed, 53 insertions(+), 17 deletions(-)
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 8b05bec08e04..24e42cb10ade 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -810,14 +810,20 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
> * folios in these areas, we need to consume the reserves
> * to keep reservation accounting consistent.
> */
> - folio = alloc_hugetlb_folio(&pseudo_vma, addr, false);
> + {
> + bool zeroed;
> +
> + folio = alloc_hugetlb_folio(&pseudo_vma, addr, false,
> + __GFP_ZERO, &zeroed);
This feels like a very odd pattern:
1) ask for __GFP_ZERO
2) Have to check whether it was actually zeroed
Seems like the zeroing piece should just be sunk in if you're going to
ask for __GFP_ZERO anyway. And in that case, maybe just `bool zero` as
an argument, rather than GFP (to avoid future overloading of flags).
~Gregory
^ permalink raw reply
* Re: [PATCH resend v6 13/30] mm: use __GFP_ZERO in vma_alloc_anon_folio_pmd
From: Gregory Price @ 2026-05-11 16:26 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett
In-Reply-To: <c8768c7c2a4965b5e2dd78680c936b82606cc034.1778489843.git.mst@redhat.com>
On Mon, May 11, 2026 at 05:02:57AM -0400, Michael S. Tsirkin wrote:
> Convert vma_alloc_anon_folio_pmd() to pass __GFP_ZERO instead of
> zeroing at the callsite. post_alloc_hook uses the fault address
> passed through vma_alloc_folio for cache-friendly zeroing.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Gregory Price <gourry@gourry.net>
^ permalink raw reply
* Re: [PATCH resend v6 12/30] mm: vma_alloc_anon_folio_pmd: pass raw fault address to vma_alloc_folio
From: Gregory Price @ 2026-05-11 16:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett
In-Reply-To: <20ce2bd8227c0149fb205d293b47348c3ef49974.1778489843.git.mst@redhat.com>
On Mon, May 11, 2026 at 05:02:51AM -0400, Michael S. Tsirkin wrote:
> Now that vma_alloc_folio aligns the address internally, drop the
> redundant HPAGE_PMD_MASK alignment at the callsite.
>
Same with prior alignment patches, seems like these should be moved
ahead.
^ permalink raw reply
* Re: [PATCH resend v6 11/30] mm: use __GFP_ZERO in alloc_anon_folio
From: Gregory Price @ 2026-05-11 16:15 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett
In-Reply-To: <99c79224ca94e63dfcbe8523cedb2a8d658bdce1.1778489843.git.mst@redhat.com>
On Mon, May 11, 2026 at 05:02:45AM -0400, Michael S. Tsirkin wrote:
> Convert alloc_anon_folio() to pass __GFP_ZERO instead of zeroing
> at the callsite. post_alloc_hook uses the fault address passed
> through vma_alloc_folio for cache-friendly zeroing.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Gregory Price <gourry@gourry.net>
^ permalink raw reply
* Re: [PATCH resend v6 10/30] mm: alloc_swap_folio: pass raw fault address to vma_alloc_folio
From: Gregory Price @ 2026-05-11 16:05 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett
In-Reply-To: <369a08b86f51572b2715a9ec577de27ea98000d3.1778489843.git.mst@redhat.com>
On Mon, May 11, 2026 at 05:02:39AM -0400, Michael S. Tsirkin wrote:
> Same change as the previous patch but for alloc_swap_folio:
> pass vmf->address directly instead of ALIGN_DOWN(vmf->address, ...).
>
Starting to seem like this should all just get squashed into once patch.
~Gregory
^ permalink raw reply
* Re: [PATCH resend v6 09/30] mm: alloc_anon_folio: pass raw fault address to vma_alloc_folio
From: Gregory Price @ 2026-05-11 16:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett
In-Reply-To: <96bbd5dcc37aed93a9b7733780bdcba4fdacfa3d.1778489843.git.mst@redhat.com>
On Mon, May 11, 2026 at 05:02:32AM -0400, Michael S. Tsirkin wrote:
> Pass vmf->address directly instead of ALIGN_DOWN(vmf->address, ...).
> vma_alloc_folio_noprof now aligns internally for NUMA interleave,
> and post_alloc_hook will use the raw address for cache-friendly
> zeroing via folio_zero_user().
>
This patch seems rather far detatched from alignment change.
I wonder whether this particular improvement should just be pulled out
ahead to shorten the series. It doesn't seem like it would cause you
too much churn?
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh
Reviewed-by: Gregory Price <gourry@gourry.net>
^ permalink raw reply
* Re: [PATCH] vhost/vsock: Refuse the connection immediately when guest isn't ready
From: Stefano Garzarella @ 2026-05-11 15:56 UTC (permalink / raw)
To: Polina Vishneva
Cc: linux-kernel, netdev, virtualization, kvm, Eugenio Pérez,
Jason Wang, Michael S. Tsirkin, Stefan Hajnoczi, Denis V . Lunev
In-Reply-To: <20260511145610.413210-1-polina.vishneva@virtuozzo.com>
On Mon, May 11, 2026 at 04:56:10PM +0200, Polina Vishneva wrote:
>From: "Denis V. Lunev" <den@openvz.org>
>
>When the host initiates an AF_VSOCK connect() to a guest that has not
>yet loaded the virtio-vsock transport (i.e. still booting), the caller
>blocks for VSOCK_DEFAULT_CONNECT_TIMEOUT (2 seconds), because
>vhost_transport_do_send_pkt() silently exits when
>vhost_vq_get_backend(vq) returns NULL.
Can SO_VM_SOCKETS_CONNECT_TIMEOUT helps on this?
>
>If the guest doesn't start listening within this timeout, connect()
>returns ETIMEDOUT.
>
>This delay is usually pointless and it doesn't well align with our
>behavior at other initialization stages: for example, if a connection is
>attempted when the guest driver is already loaded, but when nothing is
>listening yet, it returns ECONNRESET immediately without any wait.
>
>Fix this by checking the RX virtqueue backend in
>vhost_transport_send_pkt() before queuing. If the backend is NULL,
>return -ECONNREFUSED immediately.
>
>Signed-off-by: Denis V. Lunev <den@openvz.org>
>Co-developed-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
>Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
>---
> drivers/vhost/vsock.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index 1d8ec6bed53e..a3f218292c3a 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -302,6 +302,16 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
> return -ENODEV;
> }
>
>+ /* Fast-fail if the guest hasn't enabled the RX vq yet. Reading
>+ * private_data without vq->mutex is deliberate: even if the backend becomes
>+ * NULL right after that check, do_send_pkt() checks it under the mutex.
>+ */
>+ if (!data_race(READ_ONCE(vsock->vqs[VSOCK_VQ_RX].private_data)))
Why not using vhost_vq_get_backend() ?
Also is READ_ONCE() okay without WRITE_ONCE() where it is set ?
>{
>+ rcu_read_unlock();
>+ kfree_skb(skb);
>+ return -ECONNREFUSED;
This is a generic send_pkt, is it okay to return ECONNREFUSED in any
case?
Thanks,
Stefano
>+ }
>+
> if (virtio_vsock_skb_reply(skb))
> atomic_inc(&vsock->queued_replies);
>
>
>base-commit: 8ab992f815d6736b5c7a6f5fd7bfe7bc106bb3dc
>--
>2.53.0
>
^ permalink raw reply
* Re: [PATCH resend v6 03/30] mm: thread user_addr through page allocator for cache-friendly zeroing
From: Michael S. Tsirkin @ 2026-05-11 15:55 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett,
Harry Yoo, Hao Li
In-Reply-To: <agH3wceKcPNmixCj@gourry-fedora-PF4VCD3F>
On Mon, May 11, 2026 at 11:37:37AM -0400, Gregory Price wrote:
> On Mon, May 11, 2026 at 05:01:55AM -0400, Michael S. Tsirkin wrote:
> > Thread a user virtual address from vma_alloc_folio() down through
> > the page allocator to post_alloc_hook(). This is plumbing
> > preparation for a subsequent patch that will use user_addr to
> > call folio_zero_user() for cache-friendly zeroing of user pages.
> >
> > The user_addr is stored in struct alloc_context and flows through:
> > vma_alloc_folio -> folio_alloc_mpol -> __alloc_pages_mpol ->
> > __alloc_frozen_pages -> get_page_from_freelist -> prep_new_page ->
> > post_alloc_hook
>
> This is the nitty-est of all nits, but when doing this can we please
> prefer stack style?
>
> vma_alloc_folio
> folio_alloc_mpol
> __alloc_pages_mpol
> __alloc_frozen_pages
> get_page_from_freelist
> prep_new_page
> post_alloc_hook
>
> Claude has a bad habit of writing changelog changes this way, and it's
> painful for a human to try to read.
Sure.
> >
> > USER_ADDR_NONE ((unsigned long)-1) is used for non-user
> > allocations, since address 0 is a valid userspace mapping.
> >
>
> > +/*
> > + * Sentinel for user_addr: indicates a non-user allocation.
> > + * Cannot use 0 because address 0 is a valid userspace mapping.
> > + */
> > +#define USER_ADDR_NONE ((unsigned long)-1)
>
> Ehm, hm. Does -1 hold as a non-user address across all architectures?
>
> What about in linear addressing / no VM mode?
this is used on a fault. I don't think there are any faults then?
But maybe FAULT_ADDR_NONE would be clearer.
> > diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> > index 7ccbda35b9ad..ee35c5367abc 100644
> > --- a/include/linux/gfp.h
> > +++ b/include/linux/gfp.h
> > @@ -337,7 +337,7 @@ static inline struct folio *folio_alloc_noprof(gfp_t gfp, unsigned int order)
> > static inline struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
> > struct mempolicy *mpol, pgoff_t ilx, int nid)
> > {
> > - return folio_alloc_noprof(gfp, order);
> > + return __folio_alloc_noprof(gfp, order, numa_node_id(), NULL);
> > }
> > #endif
> >
>
> This change seems out of place unless i'm missing something.
>
Don't remember. Could be from a change I reverted. I'll look.
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index f24bf49be047..a999f3ead852 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -1806,7 +1806,8 @@ struct address_space *hugetlb_folio_mapping_lock_write(struct folio *folio)
> > }
> >
> > static struct folio *alloc_buddy_frozen_folio(int order, gfp_t gfp_mask,
> > - int nid, nodemask_t *nmask, nodemask_t *node_alloc_noretry)
> > + int nid, nodemask_t *nmask, nodemask_t *node_alloc_noretry,
> > + unsigned long addr)
>
> user_addr? uaddr?
ok
> > @@ -1823,7 +1824,7 @@ static struct folio *alloc_buddy_frozen_folio(int order, gfp_t gfp_mask,
> > if (alloc_try_hard)
> > gfp_mask |= __GFP_RETRY_MAYFAIL;
> >
> > - folio = (struct folio *)__alloc_frozen_pages(gfp_mask, order, nid, nmask);
> > + folio = (struct folio *)__alloc_frozen_pages(gfp_mask, order, nid, nmask, addr);
>
> Not on you, but the changes in hugetlb.c as a whole are :[
>
> We do all of this to pass USER_ADDR_NONE all over the place, but the
> alternative is having a separate function specifically for user-land
> bound allocations.
>
> So the trade off is:
> a) churn the current interface for everyone
> b) add a user_ variant and know people will just get it wrong
I was also explicitly asked not to proliferate too many new APIs.
> IIRC you said the consequence of getting wrong here is subtle corruption
> if a caller got it wrong, and this was related to cache flushing for the
> provided user address?
Yes.
> Stupid question: Does this not apply to kernel allocations as well? Or
> is it simply a matter of the cache having stale data that could leak,
> and therefore it's not a concern in-kernel?
>
> ~Gregory
Not a concern since we zero through the kernel address.
--
MST
^ permalink raw reply
* Re: [PATCH resend v6 05/30] mm: page_alloc: move prep_compound_page before post_alloc_hook
From: Gregory Price @ 2026-05-11 15:54 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <6345727fdfa76157ba0518e2148d1848e0bdbc16.1778489843.git.mst@redhat.com>
On Mon, May 11, 2026 at 05:02:06AM -0400, Michael S. Tsirkin wrote:
> Move prep_compound_page() before post_alloc_hook() in prep_new_page().
>
> The next patch adds a folio_zero_user() call to post_alloc_hook(),
> which uses folio_nr_pages() to determine how many pages to zero.
> Without compound metadata set up first, folio_nr_pages() returns 1
> for higher-order allocations, so only the first page would be zeroed.
>
> All other operations in post_alloc_hook() (arch_alloc_page, KASAN,
> debug, page owner, etc.) use raw page pointers with explicit order
> counts and are unaffected by this reordering.
>
> Note: compaction_alloc_noprof() has the opposite ordering
> (post_alloc_hook before prep_compound_page). This is fine because
> compaction always passes USER_ADDR_NONE, so folio_zero_user() is
> never called there and folio_nr_pages() is never reached inside
> post_alloc_hook().
>
We can't really know that this will stay true forever, may as well
change the ordering there to prevent a tripping hazard.
Possibly you might want to just squash this change with the next patch
and add a comment that the ordering is now explicit and required that
way to be correct.
(possibly patches 4-5-6 should just be one patch, since it's all really
the same change, but 4 seems fine by itself).
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: cursor-agent:GPT-5.4-xhigh
Reviewed-by: Gregory Price <gourry@gourry.net>
^ permalink raw reply
* Re: [PATCH resend v6 03/30] mm: thread user_addr through page allocator for cache-friendly zeroing
From: Gregory Price @ 2026-05-11 15:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett,
Harry Yoo, Hao Li
In-Reply-To: <9b53972f4854c1064b92cefc464f51949afeb83f.1778489843.git.mst@redhat.com>
On Mon, May 11, 2026 at 05:01:55AM -0400, Michael S. Tsirkin wrote:
> Thread a user virtual address from vma_alloc_folio() down through
> the page allocator to post_alloc_hook(). This is plumbing
> preparation for a subsequent patch that will use user_addr to
> call folio_zero_user() for cache-friendly zeroing of user pages.
>
> The user_addr is stored in struct alloc_context and flows through:
> vma_alloc_folio -> folio_alloc_mpol -> __alloc_pages_mpol ->
> __alloc_frozen_pages -> get_page_from_freelist -> prep_new_page ->
> post_alloc_hook
This is the nitty-est of all nits, but when doing this can we please
prefer stack style?
vma_alloc_folio
folio_alloc_mpol
__alloc_pages_mpol
__alloc_frozen_pages
get_page_from_freelist
prep_new_page
post_alloc_hook
Claude has a bad habit of writing changelog changes this way, and it's
painful for a human to try to read.
>
> USER_ADDR_NONE ((unsigned long)-1) is used for non-user
> allocations, since address 0 is a valid userspace mapping.
>
> +/*
> + * Sentinel for user_addr: indicates a non-user allocation.
> + * Cannot use 0 because address 0 is a valid userspace mapping.
> + */
> +#define USER_ADDR_NONE ((unsigned long)-1)
Ehm, hm. Does -1 hold as a non-user address across all architectures?
What about in linear addressing / no VM mode?
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 7ccbda35b9ad..ee35c5367abc 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -337,7 +337,7 @@ static inline struct folio *folio_alloc_noprof(gfp_t gfp, unsigned int order)
> static inline struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
> struct mempolicy *mpol, pgoff_t ilx, int nid)
> {
> - return folio_alloc_noprof(gfp, order);
> + return __folio_alloc_noprof(gfp, order, numa_node_id(), NULL);
> }
> #endif
>
This change seems out of place unless i'm missing something.
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index f24bf49be047..a999f3ead852 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1806,7 +1806,8 @@ struct address_space *hugetlb_folio_mapping_lock_write(struct folio *folio)
> }
>
> static struct folio *alloc_buddy_frozen_folio(int order, gfp_t gfp_mask,
> - int nid, nodemask_t *nmask, nodemask_t *node_alloc_noretry)
> + int nid, nodemask_t *nmask, nodemask_t *node_alloc_noretry,
> + unsigned long addr)
user_addr? uaddr?
> @@ -1823,7 +1824,7 @@ static struct folio *alloc_buddy_frozen_folio(int order, gfp_t gfp_mask,
> if (alloc_try_hard)
> gfp_mask |= __GFP_RETRY_MAYFAIL;
>
> - folio = (struct folio *)__alloc_frozen_pages(gfp_mask, order, nid, nmask);
> + folio = (struct folio *)__alloc_frozen_pages(gfp_mask, order, nid, nmask, addr);
Not on you, but the changes in hugetlb.c as a whole are :[
We do all of this to pass USER_ADDR_NONE all over the place, but the
alternative is having a separate function specifically for user-land
bound allocations.
So the trade off is:
a) churn the current interface for everyone
b) add a user_ variant and know people will just get it wrong
IIRC you said the consequence of getting wrong here is subtle corruption
if a caller got it wrong, and this was related to cache flushing for the
provided user address?
Stupid question: Does this not apply to kernel allocations as well? Or
is it simply a matter of the cache having stale data that could leak,
and therefore it's not a concern in-kernel?
~Gregory
^ permalink raw reply
* Re: [PATCH resend v6 02/30] mm: mempolicy: fix interleave index for unaligned VMA start
From: Michael S. Tsirkin @ 2026-05-11 15:26 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <agHu4iaL-3By0YuC@gourry-fedora-PF4VCD3F>
On Mon, May 11, 2026 at 10:59:46AM -0400, Gregory Price wrote:
> > ---
> > mm/mempolicy.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > index 6832cc68120f..39e556e3d263 100644
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -2049,7 +2049,8 @@ struct mempolicy *get_vma_policy(struct vm_area_struct *vma,
> > if (pol->mode == MPOL_INTERLEAVE ||
> > pol->mode == MPOL_WEIGHTED_INTERLEAVE) {
> > *ilx += vma->vm_pgoff >> order;
> > - *ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
> > + *ilx += (addr >> (PAGE_SHIFT + order)) -
> > + (vma->vm_start >> (PAGE_SHIFT + order));
>
> There's enough (PAGE_SHIFT + ...) spread around the kernel, i wonder if
> it's worth a define or a function (not in scope, just pondering).
>
> ~Gregory
66 of these. many are not about order. Popular ones:
PAGE_SHIFT + 3 is bits per page (for bitmaps)
PAGE_SHIFT + 1 is thread stack
definitely not in scope
^ permalink raw reply
* Re: [PATCH resend v6 02/30] mm: mempolicy: fix interleave index for unaligned VMA start
From: Michael S. Tsirkin @ 2026-05-11 15:15 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <agHu4iaL-3By0YuC@gourry-fedora-PF4VCD3F>
On Mon, May 11, 2026 at 10:59:46AM -0400, Gregory Price wrote:
> On Mon, May 11, 2026 at 05:01:49AM -0400, Michael S. Tsirkin wrote:
> > The NUMA interleave index formula (addr - vm_start) >> shift
> > gives wrong results when vm_start is not aligned to the folio
> > size: the subtraction before the shift allows low bits to
> > affect the result via borrows.
> >
> > Use (addr >> shift) - (vm_start >> shift) instead, which
> > independently aligns both values before computing the
> > difference.
> >
> > No functional change for current callers: the fix only affects
> > NUMA interleave and weighted-interleave policies. Current
> > large-order callers either pre-align the address
> > (vma_alloc_anon_folio_pmd) or do not use NUMA interleave
> > (drm_pagemap). All other callers use order 0 where the old
> > and new formulas are equivalent. However subsequent patches
> > in this series add large-order callers that pass unaligned
> > fault addresses, making this fix necessary.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> Reviewed-by: Gregory Price <gourry@gourry.net>
>
> Should this just be pulled out ahead as a fix (assuming this
> set takes more time to bake)? I get it's not causing issues today,
> but the interface is otherwise technicaly broken either way.
Sure Andrew can pick it up if he wants. I found a bunch of unrelated other
"technically broken stuff" that I deferred fixing. Will get to it
once this is done.
> > ---
> > mm/mempolicy.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > index 6832cc68120f..39e556e3d263 100644
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -2049,7 +2049,8 @@ struct mempolicy *get_vma_policy(struct vm_area_struct *vma,
> > if (pol->mode == MPOL_INTERLEAVE ||
> > pol->mode == MPOL_WEIGHTED_INTERLEAVE) {
> > *ilx += vma->vm_pgoff >> order;
> > - *ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
> > + *ilx += (addr >> (PAGE_SHIFT + order)) -
> > + (vma->vm_start >> (PAGE_SHIFT + order));
>
> There's enough (PAGE_SHIFT + ...) spread around the kernel, i wonder if
> it's worth a define or a function (not in scope, just pondering).
>
> ~Gregory
^ permalink raw reply
* Re: [PATCH resend v6 01/30] mm: move vma_alloc_folio_noprof to page_alloc.c
From: Michael S. Tsirkin @ 2026-05-11 15:09 UTC (permalink / raw)
To: Gregory Price
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli, Liam R. Howlett
In-Reply-To: <agHsFsEIU8b9RiJr@gourry-fedora-PF4VCD3F>
On Mon, May 11, 2026 at 10:47:50AM -0400, Gregory Price wrote:
> On Mon, May 11, 2026 at 05:01:44AM -0400, Michael S. Tsirkin wrote:
> > +#ifdef CONFIG_NUMA
> > +/**
> > + * vma_alloc_folio - Allocate a folio for a VMA.
> > + * @gfp: GFP flags.
> > + * @order: Order of the folio.
> > + * @vma: Pointer to VMA.
> > + * @addr: Virtual address of the allocation. Must be inside @vma.
> > + *
> > + * Allocate a folio for a specific address in @vma, using the appropriate
> > + * NUMA policy. The caller must hold the mmap_lock of the mm_struct of the
> > + * VMA to prevent it from going away. Should be used for all allocations
> > + * for folios that will be mapped into user space, excepting hugetlbfs, and
> > + * excepting where direct use of folio_alloc_mpol() is more appropriate.
> > + *
> > + * Return: The folio on success or NULL if allocation fails.
> > + */
> > +struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
> > + struct vm_area_struct *vma, unsigned long addr)
> > +{
> > + struct mempolicy *pol;
> > + pgoff_t ilx;
> > + struct folio *folio;
> > +
> > + if (vma->vm_flags & VM_DROPPABLE)
> > + gfp |= __GFP_NOWARN;
> > +
> > + pol = get_vma_policy(vma, addr, order, &ilx);
> > + folio = folio_alloc_mpol_noprof(gfp, order, pol, ilx, numa_node_id());
> > + mpol_cond_put(pol);
> > + return folio;
> > +}
> > +#else
> > +struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
> > + struct vm_area_struct *vma, unsigned long addr)
> > +{
> > + if (vma->vm_flags & VM_DROPPABLE)
> > + gfp |= __GFP_NOWARN;
> > +
> > + return folio_alloc_noprof(gfp, order);
> > +}
> > +#endif
>
> Can this ifdef not reasonably be done in a header?
It can't :(
In patch 3 this will need to call __alloc_frozen_pages_noprof, and
that's in internal.h
> I know the old path relied on basically mempolicy.c being in/out of
> scope based on CONFIG_NUMA, but we should make an attempt at keeping
> page_alloc.c clean on this if possible.
>
> ~Gregory
^ permalink raw reply
* Re: [PATCH resend v6 02/30] mm: mempolicy: fix interleave index for unaligned VMA start
From: Gregory Price @ 2026-05-11 14:59 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <41bd50fdaae943e1a79215ea4000322e74a823be.1778489843.git.mst@redhat.com>
On Mon, May 11, 2026 at 05:01:49AM -0400, Michael S. Tsirkin wrote:
> The NUMA interleave index formula (addr - vm_start) >> shift
> gives wrong results when vm_start is not aligned to the folio
> size: the subtraction before the shift allows low bits to
> affect the result via borrows.
>
> Use (addr >> shift) - (vm_start >> shift) instead, which
> independently aligns both values before computing the
> difference.
>
> No functional change for current callers: the fix only affects
> NUMA interleave and weighted-interleave policies. Current
> large-order callers either pre-align the address
> (vma_alloc_anon_folio_pmd) or do not use NUMA interleave
> (drm_pagemap). All other callers use order 0 where the old
> and new formulas are equivalent. However subsequent patches
> in this series add large-order callers that pass unaligned
> fault addresses, making this fix necessary.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Gregory Price <gourry@gourry.net>
Should this just be pulled out ahead as a fix (assuming this
set takes more time to bake)? I get it's not causing issues today,
but the interface is otherwise technicaly broken either way.
> ---
> mm/mempolicy.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 6832cc68120f..39e556e3d263 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2049,7 +2049,8 @@ struct mempolicy *get_vma_policy(struct vm_area_struct *vma,
> if (pol->mode == MPOL_INTERLEAVE ||
> pol->mode == MPOL_WEIGHTED_INTERLEAVE) {
> *ilx += vma->vm_pgoff >> order;
> - *ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
> + *ilx += (addr >> (PAGE_SHIFT + order)) -
> + (vma->vm_start >> (PAGE_SHIFT + order));
There's enough (PAGE_SHIFT + ...) spread around the kernel, i wonder if
it's worth a define or a function (not in scope, just pondering).
~Gregory
^ permalink raw reply
* [PATCH v5 3/3] vfio/pci: Replace vfio_pci_core_setup_barmap() with vfio_pci_core_get_iomap()
From: Matt Evans @ 2026-05-11 14:58 UTC (permalink / raw)
To: Alex Williamson, Kevin Tian, Jason Gunthorpe, Ankit Agrawal,
Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum,
Yishai Hadas
Cc: Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy,
Zhi Wang, kvm, linux-kernel, virtualization
In-Reply-To: <20260511145829.2993601-1-mattev@meta.com>
Since "vfio/pci: Set up barmap in vfio_pci_core_enable()", the
resource request and iomap for the BARs was performed early, and
vfio_pci_core_setup_barmap() just checks those actions succeeded.
Move this logic to a new helper that checks success and returns the
iomap address, replacing the various bare vdev->barmap[] lookups.
This maintains the error behaviour of the previous on-demand
vfio_pci_core_setup_barmap() scheme.
Signed-off-by: Matt Evans <mattev@meta.com>
---
drivers/vfio/pci/nvgrace-gpu/main.c | 20 ++++++++++++-------
drivers/vfio/pci/vfio_pci_core.c | 11 +++++------
drivers/vfio/pci/vfio_pci_dmabuf.c | 2 +-
drivers/vfio/pci/vfio_pci_rdwr.c | 30 ++++++++---------------------
drivers/vfio/pci/virtio/legacy_io.c | 13 ++++++-------
include/linux/vfio_pci_core.h | 20 ++++++++++++++++++-
6 files changed, 52 insertions(+), 44 deletions(-)
diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c
index fa056b69f899..15e2f03c6cd4 100644
--- a/drivers/vfio/pci/nvgrace-gpu/main.c
+++ b/drivers/vfio/pci/nvgrace-gpu/main.c
@@ -61,6 +61,7 @@ struct nvgrace_gpu_pci_core_device {
struct mem_region resmem;
/* Lock to control device memory kernel mapping */
struct mutex remap_lock;
+ void __iomem *bar0_base;
bool has_mig_hw_bug;
/* GPU has just been reset */
bool reset_done;
@@ -171,6 +172,7 @@ static int nvgrace_gpu_open_device(struct vfio_device *core_vdev)
struct nvgrace_gpu_pci_core_device *nvdev =
container_of(core_vdev, struct nvgrace_gpu_pci_core_device,
core_device.vdev);
+ void __iomem *io;
int ret;
ret = vfio_pci_core_enable(vdev);
@@ -184,14 +186,14 @@ static int nvgrace_gpu_open_device(struct vfio_device *core_vdev)
/*
* GPU readiness is checked by reading the BAR0 registers.
- *
- * ioremap BAR0 to ensure that the BAR0 mapping is present before
- * register reads on first fault before establishing any GPU
- * memory mapping.
+ * The BAR map was just set up by vfio_pci_core_enable(), so
+ * bail early if that wasn't successful:
*/
- ret = vfio_pci_core_setup_barmap(vdev, 0);
- if (ret)
+ io = vfio_pci_core_get_iomap(vdev, 0);
+ if (IS_ERR(io)) {
+ ret = PTR_ERR(io);
goto error_exit;
+ }
if (nvdev->resmem.memlength) {
ret = nvgrace_gpu_vfio_pci_register_pfn_range(core_vdev, &nvdev->resmem);
@@ -204,6 +206,8 @@ static int nvgrace_gpu_open_device(struct vfio_device *core_vdev)
goto register_mem_failed;
vfio_pci_core_finish_enable(vdev);
+ nvdev->bar0_base = io;
+
return 0;
register_mem_failed:
@@ -220,6 +224,8 @@ static void nvgrace_gpu_close_device(struct vfio_device *core_vdev)
container_of(core_vdev, struct nvgrace_gpu_pci_core_device,
core_device.vdev);
+ nvdev->bar0_base = NULL;
+
if (nvdev->resmem.memlength)
unregister_pfn_address_space(&nvdev->resmem.pfn_address_space);
@@ -275,7 +281,7 @@ nvgrace_gpu_check_device_ready(struct nvgrace_gpu_pci_core_device *nvdev)
if (!__vfio_pci_memory_enabled(vdev))
return -EIO;
- ret = nvgrace_gpu_wait_device_ready(vdev->barmap[0]);
+ ret = nvgrace_gpu_wait_device_ready(nvdev->bar0_base);
if (ret)
return ret;
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 0e4fc17d360f..b1b7fc5183a7 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1762,7 +1762,7 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma
struct pci_dev *pdev = vdev->pdev;
unsigned int index;
u64 phys_len, req_len, pgoff, req_start;
- int ret;
+ void __iomem *bar_io;
index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
@@ -1796,12 +1796,11 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma
return -EINVAL;
/*
- * Even though we don't make use of the barmap for the mmap,
- * we need to request the region and the barmap tracks that.
+ * Ensure the BAR resource region is reserved for use.
*/
- ret = vfio_pci_core_setup_barmap(vdev, index);
- if (ret)
- return ret;
+ bar_io = vfio_pci_core_get_iomap(vdev, index);
+ if (IS_ERR(bar_io))
+ return PTR_ERR(bar_io);
vma->vm_private_data = vdev;
vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
index 69a5c2d511e6..46cd44b22c9c 100644
--- a/drivers/vfio/pci/vfio_pci_dmabuf.c
+++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
@@ -248,7 +248,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
* else. Check that PCI resources have been claimed for it.
*/
if (get_dma_buf.region_index >= VFIO_PCI_ROM_REGION_INDEX ||
- vfio_pci_core_setup_barmap(vdev, get_dma_buf.region_index))
+ IS_ERR(vfio_pci_core_get_iomap(vdev, get_dma_buf.region_index)))
return -ENODEV;
dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges,
diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index 3bfbb879a005..7f14dd46de17 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c
@@ -198,19 +198,6 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
}
EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw);
-/*
- * The barmap is set up in vfio_pci_core_enable(). Callers use this
- * function to check that the BAR resources are requested or that the
- * pci_iomap() was done.
- */
-int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar)
-{
- if (IS_ERR(vdev->barmap[bar]))
- return PTR_ERR(vdev->barmap[bar]);
- return 0;
-}
-EXPORT_SYMBOL_GPL(vfio_pci_core_setup_barmap);
-
ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
size_t count, loff_t *ppos, bool iswrite)
{
@@ -262,13 +249,11 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
*/
max_width = VFIO_PCI_IO_WIDTH_4;
} else {
- int ret = vfio_pci_core_setup_barmap(vdev, bar);
- if (ret) {
- done = ret;
+ io = vfio_pci_core_get_iomap(vdev, bar);
+ if (IS_ERR(io)) {
+ done = PTR_ERR(io);
goto out;
}
-
- io = vdev->barmap[bar];
}
if (bar == vdev->msix_bar) {
@@ -423,6 +408,7 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset,
loff_t pos = offset & VFIO_PCI_OFFSET_MASK;
int ret, bar = VFIO_PCI_OFFSET_TO_INDEX(offset);
struct vfio_pci_ioeventfd *ioeventfd;
+ void __iomem *io;
/* Only support ioeventfds into BARs */
if (bar > VFIO_PCI_BAR5_REGION_INDEX)
@@ -440,9 +426,9 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset,
if (count == 8)
return -EINVAL;
- ret = vfio_pci_core_setup_barmap(vdev, bar);
- if (ret)
- return ret;
+ io = vfio_pci_core_get_iomap(vdev, bar);
+ if (IS_ERR(io))
+ return PTR_ERR(io);
mutex_lock(&vdev->ioeventfds_lock);
@@ -479,7 +465,7 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset,
}
ioeventfd->vdev = vdev;
- ioeventfd->addr = vdev->barmap[bar] + pos;
+ ioeventfd->addr = io + pos;
ioeventfd->data = data;
ioeventfd->pos = pos;
ioeventfd->bar = bar;
diff --git a/drivers/vfio/pci/virtio/legacy_io.c b/drivers/vfio/pci/virtio/legacy_io.c
index 1ed349a55629..c868b2177310 100644
--- a/drivers/vfio/pci/virtio/legacy_io.c
+++ b/drivers/vfio/pci/virtio/legacy_io.c
@@ -299,19 +299,18 @@ int virtiovf_pci_ioctl_get_region_info(struct vfio_device *core_vdev,
static int virtiovf_set_notify_addr(struct virtiovf_pci_core_device *virtvdev)
{
struct vfio_pci_core_device *core_device = &virtvdev->core_device;
- int ret;
+ void __iomem *io;
/*
* Setup the BAR where the 'notify' exists to be used by vfio as well
* This will let us mmap it only once and use it when needed.
*/
- ret = vfio_pci_core_setup_barmap(core_device,
- virtvdev->notify_bar);
- if (ret)
- return ret;
+ io = vfio_pci_core_get_iomap(core_device,
+ virtvdev->notify_bar);
+ if (IS_ERR(io))
+ return PTR_ERR(io);
- virtvdev->notify_addr = core_device->barmap[virtvdev->notify_bar] +
- virtvdev->notify_offset;
+ virtvdev->notify_addr = io + virtvdev->notify_offset;
return 0;
}
diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index 89165b769e5c..7accd0eac457 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -188,7 +188,6 @@ int vfio_pci_core_match_token_uuid(struct vfio_device *core_vdev,
int vfio_pci_core_enable(struct vfio_pci_core_device *vdev);
void vfio_pci_core_disable(struct vfio_pci_core_device *vdev);
void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev);
-int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar);
pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev,
pci_channel_state_t state);
ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
@@ -234,6 +233,25 @@ static inline bool is_aligned_for_order(struct vm_area_struct *vma,
!IS_ALIGNED(pfn, 1 << order)));
}
+/*
+ * Returns a BAR's iomap base or an ERR_PTR() if, for example, the
+ * BAR isn't valid, its resource wasn't acquired, or its iomap
+ * failed. This shall only be used after vfio_pci_core_enable()
+ * has set up the BAR maps and before vfio_pci_core_disable()
+ * tears them down.
+ */
+static inline void __iomem __must_check *
+vfio_pci_core_get_iomap(struct vfio_pci_core_device *vdev, unsigned int bar)
+{
+ if (WARN_ON_ONCE(bar >= PCI_STD_NUM_BARS))
+ return ERR_PTR(-EINVAL);
+
+ if (WARN_ON_ONCE(!vdev->barmap[bar]))
+ return ERR_PTR(-ENODEV);
+
+ return vdev->barmap[bar];
+}
+
int vfio_pci_dma_buf_iommufd_map(struct dma_buf_attachment *attachment,
struct phys_vec *phys);
--
2.47.3
^ permalink raw reply related
* [PATCH v5 2/3] vfio/pci: Check BAR resources before exporting a DMABUF
From: Matt Evans @ 2026-05-11 14:58 UTC (permalink / raw)
To: Alex Williamson, Kevin Tian, Jason Gunthorpe, Ankit Agrawal,
Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum,
Yishai Hadas
Cc: Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy,
Zhi Wang, kvm, linux-kernel, virtualization
In-Reply-To: <20260511145829.2993601-1-mattev@meta.com>
A DMABUF exports access to BAR resources and, although they are
requested at startup time, we need to ensure they really were reserved
before exporting. Otherwise, it's possible to access unreserved
resources through the export.
Add a check to the DMABUF-creation path.
Fixes: 5d74781ebc86c ("vfio/pci: Add dma-buf export support for MMIO regions")
Signed-off-by: Matt Evans <mattev@meta.com>
---
drivers/vfio/pci/vfio_pci_dmabuf.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
index f87fd32e4a01..69a5c2d511e6 100644
--- a/drivers/vfio/pci/vfio_pci_dmabuf.c
+++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
@@ -244,9 +244,11 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
return -EINVAL;
/*
- * For PCI the region_index is the BAR number like everything else.
+ * For PCI the region_index is the BAR number like everything
+ * else. Check that PCI resources have been claimed for it.
*/
- if (get_dma_buf.region_index >= VFIO_PCI_ROM_REGION_INDEX)
+ if (get_dma_buf.region_index >= VFIO_PCI_ROM_REGION_INDEX ||
+ vfio_pci_core_setup_barmap(vdev, get_dma_buf.region_index))
return -ENODEV;
dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges,
--
2.47.3
^ permalink raw reply related
* [PATCH v5 1/3] vfio/pci: Set up BAR resources and maps in vfio_pci_core_enable()
From: Matt Evans @ 2026-05-11 14:58 UTC (permalink / raw)
To: Alex Williamson, Kevin Tian, Jason Gunthorpe, Ankit Agrawal,
Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum,
Yishai Hadas
Cc: Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy,
Zhi Wang, kvm, linux-kernel, virtualization
In-Reply-To: <20260511145829.2993601-1-mattev@meta.com>
Previously BAR resource requests and the corresponding pci_iomap()
were performed on-demand and without synchronisation, which was racy.
Rather than add synchronisation, it's simplest to address this by
doing both activities from vfio_pci_core_enable().
The resource allocation and/or pci_iomap() can still fail; their
status is tracked and existing calls to vfio_pci_core_setup_barmap()
will fail in a similar way to before. This keeps the point of failure
as observed by userspace the same, i.e. failures to request/map unused
BARs are benign.
Fixes: 89e1f7d4c66d ("vfio: Add PCI device driver")
Signed-off-by: Matt Evans <mattev@meta.com>
---
drivers/vfio/pci/vfio_pci_core.c | 37 +++++++++++++++++++++++++++++++-
drivers/vfio/pci/vfio_pci_rdwr.c | 26 ++++++----------------
2 files changed, 43 insertions(+), 20 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 3f8d093aacf8..0e4fc17d360f 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -482,6 +482,40 @@ static int vfio_pci_core_runtime_resume(struct device *dev)
}
#endif /* CONFIG_PM */
+/*
+ * Eager-request BAR resources, and iomap them. Soft failures are
+ * allowed, and consumers must check the barmap before use in order to
+ * give compatible user-visible behaviour with the previous on-demand
+ * allocation method.
+ */
+static void vfio_pci_core_map_bars(struct vfio_pci_core_device *vdev)
+{
+ struct pci_dev *pdev = vdev->pdev;
+ int i;
+
+ for (i = 0; i < PCI_STD_NUM_BARS; i++) {
+ int bar = i + PCI_STD_RESOURCES;
+
+ vdev->barmap[bar] = ERR_PTR(-ENODEV);
+
+ if (!pci_resource_len(pdev, i))
+ continue;
+
+ if (pci_request_selected_regions(pdev, 1 << bar, "vfio")) {
+ pci_dbg(pdev, "Failed to reserve region %d\n", bar);
+ vdev->barmap[bar] = ERR_PTR(-EBUSY);
+ continue;
+ }
+
+ vdev->barmap[bar] = pci_iomap(pdev, bar, 0);
+ if (!vdev->barmap[bar]) {
+ pci_dbg(pdev, "Failed to iomap region %d\n", bar);
+ pci_release_selected_regions(pdev, 1 << bar);
+ vdev->barmap[bar] = ERR_PTR(-ENOMEM);
+ }
+ }
+}
+
/*
* The pci-driver core runtime PM routines always save the device state
* before going into suspended state. If the device is going into low power
@@ -568,6 +602,7 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
if (!vfio_vga_disabled() && vfio_pci_is_vga(pdev))
vdev->has_vga = true;
+ vfio_pci_core_map_bars(vdev);
return 0;
@@ -648,7 +683,7 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
for (i = 0; i < PCI_STD_NUM_BARS; i++) {
bar = i + PCI_STD_RESOURCES;
- if (!vdev->barmap[bar])
+ if (IS_ERR_OR_NULL(vdev->barmap[bar]))
continue;
pci_iounmap(pdev, vdev->barmap[bar]);
pci_release_selected_regions(pdev, 1 << bar);
diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index 4251ee03e146..3bfbb879a005 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c
@@ -198,27 +198,15 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
}
EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw);
+/*
+ * The barmap is set up in vfio_pci_core_enable(). Callers use this
+ * function to check that the BAR resources are requested or that the
+ * pci_iomap() was done.
+ */
int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar)
{
- struct pci_dev *pdev = vdev->pdev;
- int ret;
- void __iomem *io;
-
- if (vdev->barmap[bar])
- return 0;
-
- ret = pci_request_selected_regions(pdev, 1 << bar, "vfio");
- if (ret)
- return ret;
-
- io = pci_iomap(pdev, bar, 0);
- if (!io) {
- pci_release_selected_regions(pdev, 1 << bar);
- return -ENOMEM;
- }
-
- vdev->barmap[bar] = io;
-
+ if (IS_ERR(vdev->barmap[bar]))
+ return PTR_ERR(vdev->barmap[bar]);
return 0;
}
EXPORT_SYMBOL_GPL(vfio_pci_core_setup_barmap);
--
2.47.3
^ permalink raw reply related
* [PATCH v5 0/3] vfio/pci: Request resources and map BARs at enable time
From: Matt Evans @ 2026-05-11 14:58 UTC (permalink / raw)
To: Alex Williamson, Kevin Tian, Jason Gunthorpe, Ankit Agrawal,
Alistair Popple, Leon Romanovsky, Kees Cook, Shameer Kolothum,
Yishai Hadas
Cc: Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy,
Zhi Wang, kvm, linux-kernel, virtualization
Hi,
These patches fix a potential race for concurrent calls to
vfio_pci_core_setup_barmap(), and a DMABUF missing check for resource
before the export. Discussion on a previous series (different,
replaced by this one) is here:
https://lore.kernel.org/kvm/20260415181423.1008458-1-mattev@meta.com
Responses in that thread indicated there wasn't a strong historical
reason to require the mapping to be performed on-demand at BAR
reference time. It's much simpler to move this earlier, to
vfio_pci_core_enable(), and that then avoids having to deal with
concurrent requests later.
The first patch requests PCI resources and pci_iomap() of the BARs
from vfio_pci_core_enable(), moving this out of
vfio_pci_core_setup_barmap().
Some callers rely on vfio_pci_core_setup_barmap() for its ioremap()
effect, and other callers use it for its resource-acquiring effect.
The function turns into a cheap error check that both these actions
have occurred; that maintains the same error behaviour as before the
fix.
The second patch adds a call to vfio_pci_core_setup_barmap() to VFIO
DMABUF export to check the resource is reserved; previously this was
able to export an unrequested resource. Although patch 1 at first
appears to fix this by requesting resources at enable time, code using
the BAR still needs to check the resource really was acquired.
(FWIW, Leon gave a R-B on v2 in
https://lore.kernel.org/kvm/20260426111606.GC440345@unreal/ though
this repost isn't 100% identical so I didn't propagate it.)
The third patch refactors vfio_pci_core_setup_barmap() plus the various
vdev->barmap[] accesses into vfio_pci_core_get_iomap() which returns
either a pointer to the mapping or an ERR_PTR() describing why it
doesn't exist. This is used by callers that need the mapping, but
also by other callers to check that the resource/mapping step was
successful.
=== Changes ===
v5:
- vfio_pci_core_map_bars() releases the region if the subsequent
pci_iomap() fails
- vfio_pci_core_disable() correctly tests for NULL/error
- For nvgrace-gpu, check the result of vfio_pci_core_get_iomap() at
open time then stash the BAR base for use later on. Errors are
checked up-front, and this avoids the perception that a
later ..._get_iomap() might fail.
v4:
https://lore.kernel.org/kvm/20260505173835.2324179-1-mattev@meta.com/
- Reorder patches to put fixes at the front: First, the early BAR
setup to avoid the race. Then, add DMABUF check. Then,
refactor/tidy.
- Adjust Fixes: of first patch to point to early VFIO PCI commit, and
reduce the patch to only the fix (don't add new error checks).
Use pci_dbg() instead of pci_warn() when setting up BAR
resources. Add barmap[] error checking to vfio_pci_core_disable().
- Add barmap[]/BAR index error checking to vfio_pci_core_get_iomap(),
and use WARN_ON_ONCE() since the conditions truly shouldn't happen.
v3:
https://lore.kernel.org/kvm/20260430100340.2787446-1-mattev@meta.com/
- Remove the separate tracking of the BAR mapping versus the
acquiring its resource. Errors from failing iomap vs resource
reservation are ERR_PTR()-elcoded into barmap[bar].
- Remove the separate test helper, and add vfio_pci_core_get_iomap().
This gets the iomap base or is used check for error/failure to
acquire the resource. Added comments at call sites explaining
whether they want to just ensure the resource is reserved versus
actually use the mapping.
v2:
https://lore.kernel.org/kvm/20260423182517.2286030-1-mattev@meta.com/
- Don't fail if resources can't be requested or iomapped, even for
valid BARs, as this would change the userspace-observable error
behaviour. Specifically, if there was an issue with one particular
BAR which happened to never be used, then userspace would never
encounter an error for it. Track iomap and resource-acquisition
status per BAR.
- Break out the checks for resource success from those for iomap
success, in the form of the two new helpers.
- Third patch to add the check to VFIO DMABUF export, because
init-time requests can now fail.
v1:
https://lore.kernel.org/kvm/20260421174143.3883579-1-mattev@meta.com/
Matt Evans (3):
vfio/pci: Set up BAR resources and maps in vfio_pci_core_enable()
vfio/pci: Check BAR resources before exporting a DMABUF
vfio/pci: Replace vfio_pci_core_setup_barmap() with
vfio_pci_core_get_iomap()
drivers/vfio/pci/nvgrace-gpu/main.c | 20 +++++++-----
drivers/vfio/pci/vfio_pci_core.c | 48 ++++++++++++++++++++++++-----
drivers/vfio/pci/vfio_pci_dmabuf.c | 6 ++--
drivers/vfio/pci/vfio_pci_rdwr.c | 42 +++++--------------------
drivers/vfio/pci/virtio/legacy_io.c | 13 ++++----
include/linux/vfio_pci_core.h | 20 +++++++++++-
6 files changed, 91 insertions(+), 58 deletions(-)
--
2.47.3
^ 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