On 08.07.25 04:52, Hugh Dickins wrote:
On Mon, 7 Jul 2025, David Hildenbrand wrote:
On 07.07.25 08:31, Hugh Dickins wrote:
On Fri, 4 Jul 2025, David Hildenbrand wrote:
On 03.07.25 16:44, Lance Yang wrote:
On 2025/7/3 20:39, David Hildenbrand wrote:
On 03.07.25 14:34, Lance Yang wrote:
On Mon, Jun 23, 2025 at 10:04 PM David Hildenbrand <david@xxxxxxxxxx>
wrote:
On 20.06.25 14:50, Oscar Salvador wrote:
On Tue, Jun 17, 2025 at 05:43:32PM +0200, David Hildenbrand wrote:
In 2009, we converted a VM_BUG_ON(!pfn_valid(pfn)) to the current
highest_memmap_pfn sanity check in commit 22b31eec63e5 ("badpage:
vm_normal_page use print_bad_pte"), because highest_memmap_pfn was
readily available.
highest_memmap_pfn was introduced by that commit for this purpose.
Oh, somehow I thought it was around before that.
[...]
We catch corruption in a handful of PTE bits, and that's about it. You neither
detect corruption of flags nor of PFN bits that result in another valid PFN.
Of course it's limited in what it can catch (and won't even get called
if the present bit was not set - a more complete patch might unify with
those various "Bad swap" messages). Of course. But it's still useful for
stopping pfn_to_page() veering off the end of the memmap[] (in some configs).
Right, probably in the configs we both don't care that much about
nowadays :)
And it's still useful for printing out a series of "Bad page map" messages
when the page table is corrupted: from which a picture can sometimes be
built up (isolated instance may just be a bitflip; series of them can
sometimes show e.g. ascii text, occasionally helpful for debugging).
It's kind of a weird thing, because we do something very different
opposed to other areas where we detect that something serious is going
wrong (e.g., WARN).
But another thread just sparked whether we should WARN here, so I'll
leave that discussion to the other thread.
Corruption of the "special" bit might be fun.
When I was able to trigger this during development once, the whole machine
went down shortly after -- mostly because of use-after-free of something that
is now a page table, which is just bad for both users of such a page!
E.g., quit that process and we will happily clear the PTE, corrupting data of
the other user. Fun.
I'm sure I could find a way to unify the code while printing some comparable
message, but this check as it stands is just not worth it IMHO: trying to
handle something gracefully that shouldn't happen, when really we cannot
handle it gracefully.
So, you have experience of a time when it didn't help you. Okay. And we
have had experience of other times when it has helped, if only a little.
Like with other "Bad page"s: sometimes helpful, often not; but tending to
build up a big picture from repeated occurrences.
Okay. I was rather curious how often we would actually hit this one
here: from my recollection, the mapcount underflows are much more
frequent than the ones from vm_normal_page().
We continue to disagree. I can't argue more than append the 2.6.29
commit message, which seems to me as valid now as it was then.
Not that I agree that performing these sanity checks in each and every
config is something reasonable, but apparently you think that they are
still useful, 16 years after they were introduced.
So, let me try finding a way to unify the code while keeping that error
handling for now in place.
--
Cheers,
David / dhildenb
