Re: [mm 4.15-rc8] Random oopses under memory pressure.

[Kirill A. Shutemov]

On Thu, Jan 18, 2018 at 05:12:45PM +0900, Tetsuo Handa wrote:
> Tetsuo Handa wrote:
> > OK. I missed the mark. I overlooked that 4.11 already has this problem.
> > 
> > I needed to bisect between 4.10 and 4.11, and I got plausible culprit.
> > 
> > I haven't completed bisecting between b4fb8f66f1ae2e16 and c470abd4fde40ea6, but
> > b4fb8f66f1ae2e16 ("mm, page_alloc: Add missing check for memory holes") and
> > 13ad59df67f19788 ("mm, page_alloc: avoid page_to_pfn() when merging buddies")
> > are talking about memory holes, which matches the situation that I'm trivially
> > hitting the bug if CONFIG_SPARSEMEM=y .
> > 
> > Thus, I call for an attention by speculative execution. ;-)
> 
> Speculative execution failed. I was confused by jiffies precision bug.
> The final culprit is c7ab0d2fdc840266 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()").

I think I've tracked it down. check_pte() in mm/page_vma_mapped.c doesn't
work as intended.

I've added instrumentation below to prove it.

The BUG() triggers with following output:

[   10.084024] diff: -858690919
[   10.084258] hpage_nr_pages: 1
[   10.084386] check1: 0
[   10.084478] check2: 0

Basically, pte_page(*pvmw->pte) is below pvmw->page, but
(pte_page(*pvmw->pte) < pvmw->page) doesn't catch it.

Well, I can see how C lawyer can argue that you can only compare pointers
of the same memory object which is not the case here. But this is kinda
insane.

Any suggestions how to rewrite it in a way that compiler would
understand?

diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index d22b84310f6d..57b4397f1ea5 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -70,6 +70,14 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
 		}
 		if (pte_page(*pvmw->pte) < pvmw->page)
 			return false;
+
+		if (pte_page(*pvmw->pte) - pvmw->page) {
+			printk("diff: %d\n", pte_page(*pvmw->pte) - pvmw->page);
+			printk("hpage_nr_pages: %d\n", hpage_nr_pages(pvmw->page));
+			printk("check1: %d\n", pte_page(*pvmw->pte) - pvmw->page < 0);
+			printk("check2: %d\n", pte_page(*pvmw->pte) - pvmw->page >= hpage_nr_pages(pvmw->page));
+			BUG();
+		}
 	}
 
 	return true;
-- 
 Kirill A. Shutemov