[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHk-=wiCYfNp4AJLBORU-c7ZyRBUp66W2-Et6cdQ4REx-GyQ_A@mail.gmail.com>
Date: Tue, 5 Aug 2025 16:00:53 +0300
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: David Hildenbrand <david@...hat.com>
Cc: Alex Williamson <alex.williamson@...hat.com>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"lizhe.67@...edance.com" <lizhe.67@...edance.com>, Jason Gunthorpe <jgg@...dia.com>
Subject: Re: [GIT PULL] VFIO updates for v6.17-rc1
On Tue, 5 Aug 2025 at 10:47, David Hildenbrand <david@...hat.com> wrote:
>
> The concern is rather false positives, meaning, you want consecutive
> PFNs (just like within a folio), but -- because the stars aligned --
> you get consecutive "struct page" that do not translate to consecutive PFNs.
So I don't think that can happen with a valid 'struct page', because
if the 'struct page's are in different sections, they will have been
allocated separately too.
So you can't have two consecutive 'struct page' things without them
being consecutive pages.
But by all means, if you want to make sure, just compare the page
sections. But converting them to a PFN and then converting back is
just crazy.
IOW, the logic would literally be something like (this assumes there
is always at least *one* page):
struct page *page = *pages++;
int section = page_to_section(page);
for (size_t nr = 1; nr < nr_pages; nr++) {
if (*pages++ != ++page)
break;
if (page_to_section(page) != section)
break;
}
return nr;
and yes, I think we only define page_to_section() for
SECTION_IN_PAGE_FLAGS, but we should fix that and just have a
#define page_to_section(pg) 0
for the other cases, and the compiler will happily optimize away the
"oh, it's always zero" case.
So something like that should actually generate reasonable code. It
*really* shouldn't try to generate a pfn (or, like that horror that I
didn't pull did, then go *back* from pfn to page)
That 'nth_page()' thing is just crazy garbage.
And even when fixed to not be garbage, I'm not convinced this needs to
be in <linux/mm.h>.
Linus
PS. No - I didn't test the above trivial loop. It may be trivial, but
it may be buggy. Think of it as "something like this" rather than
"this is tested and does the right thing"
Powered by blists - more mailing lists