lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 5 Dec 2020 21:26:47 -0500
From:   Andrea Arcangeli <aarcange@...hat.com>
To:     Mel Gorman <mgorman@...e.de>
Cc:     Vlastimil Babka <vbabka@...e.cz>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        Qian Cai <cai@....pw>, Michal Hocko <mhocko@...nel.org>,
        David Hildenbrand <david@...hat.com>,
        linux-kernel@...r.kernel.org, Mike Rapoport <rppt@...ux.ibm.com>,
        Baoquan He <bhe@...hat.com>
Subject: Re: [PATCH 1/1] mm: compaction: avoid fast_isolate_around() to set
 pageblock_skip on reserved pages

Hi Mel,

On Thu, Nov 26, 2020 at 10:47:20AM +0000, Mel Gorman wrote:
> Agreed. This thread has a lot of different directions in it at this
> point so what I'd hope for is first, a patch that initialises holes with
> zone/node linkages within a 1<<(MAX_ORDER-1) alignment. If there is a
> hole, it would be expected the pages are PageReserved. Second, a fix to
> fast_isolate that forces PFNs returned to always be within the stated
> zone boundaries.

The last two patches should resolve the struct page
initialization
https://git.kernel.org/pub/scm/linux/kernel/git/andrea/aa.git/ and the
VM_BUG_ON_PAGE never happened again as expected.

So I looked back to see how the "distance" logic is accurate. I added
those checks and I get negative hits:

diff --git a/mm/compaction.c b/mm/compaction.c
index cc1a7f600a86..844a90b0acdf 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1331,6 +1331,12 @@ fast_isolate_freepages(struct compact_control *cc)
 	low_pfn = pageblock_start_pfn(cc->free_pfn - (distance >> 2));
 	min_pfn = pageblock_start_pfn(cc->free_pfn - (distance >> 1));
 
+	WARN_ON_ONCE((long) low_pfn < 0);
+	WARN_ON_ONCE((long) min_pfn < 0);
+	if ((long) low_pfn < 0)
+		return cc->free_pfn;
+	if ((long) min_pfn < 0)
+		return cc->free_pfn;
 	if (WARN_ON_ONCE(min_pfn > low_pfn))
 		low_pfn = min_pfn;

Both warn-on-once triggers, so it goes negative. While it appears not
kernel crashing since pfn_valid won't succeed on negative entries and
they'll always be higher than any pfn in the freelist, is this sign
that there's further room for improvement here?

Thanks,
Andrea

Powered by blists - more mailing lists