[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LSU.2.11.1510221510420.1795@eggly.anvils>
Date: Thu, 22 Oct 2015 15:26:58 -0700 (PDT)
From: Hugh Dickins <hughd@...gle.com>
To: Hugh Dickins <hughd@...gle.com>
cc: Minchan Kim <minchan@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
"Kirill A. Shutemov" <kirill@...temov.name>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Rik van Riel <riel@...hat.com>,
Mel Gorman <mgorman@...e.de>, Michal Hocko <mhocko@...e.cz>,
Johannes Weiner <hannes@...xchg.org>,
Vlastimil Babka <vbabka@...e.cz>
Subject: Re: kernel oops on mmotm-2015-10-15-15-20
On Wed, 21 Oct 2015, Hugh Dickins wrote:
> On Wed, 21 Oct 2015, Hugh Dickins wrote:
> > On Thu, 22 Oct 2015, Minchan Kim wrote:
> > > Hello Hugh,
> > >
> > > On Wed, Oct 21, 2015 at 05:59:59PM -0700, Hugh Dickins wrote:
> > > > On Thu, 22 Oct 2015, Minchan Kim wrote:
> > > > >
> > > > > I added the code to check it and queued it again but I had another oops
> > > > > in this time but symptom is related to anon_vma, too.
> > > > > (kernel is based on recent mmotm + unconditional mkdirty for bug fix)
> > > > > It seems page_get_anon_vma returns NULL since the page was not page_mapped
> > > > > at that time but second check of page_mapped right before try_to_unmap seems
> > > > > to be true.
> > > > >
> > > > > Adding 4191228k swap on /dev/vda5. Priority:-1 extents:1 across:4191228k FS
> > > > > Adding 4191228k swap on /dev/vda5. Priority:-1 extents:1 across:4191228k FS
> > > > > page:ffffea0001cfbfc0 count:3 mapcount:1 mapping:ffff88007f1b5f51 index:0x600000aff
> > > > > flags: 0x4000000000048019(locked|uptodate|dirty|swapcache|swapbacked)
> > > > > page dumped because: VM_BUG_ON_PAGE(PageAnon(page) && !PageKsm(page) && !anon_vma)
> > > >
> > > > That's interesting, that's one I added in my page migration series.
> > > > Let me think on it, but it could well relate to the one you got before.
>
> I think I have introduced a bug there; or rather, made more evident
> a pre-existing bug. But I'm not sure yet: the stacktrace was from
> compaction (called by khugepaged, but that may not be relevant at all),
> and thinking through the races with isolate_migratepages_block() is
> never easy.
>
> What's certain is that I was not giving any thought to
> isolate_migratepages_block() when I added that VM_BUG_ON_PAGE():
> I was thinking about "stable" anonymous pages, and how they get
> faulted back in from swapcache while holding page lock.
>
> It looks to me now as if a page might not yet be PageAnon when it's
> first tested in __unmap_and_move(), when going to page_get_anon_vma();
> but is page_mapped() and PageAnon() by time of calling try_to_unmap(),
> where I inserted the VM_BUG_ON_PAGE().
>
> If so, the code would always have been wrong (trying to unmap the
> anonymous page, and later remap its replacement, without a hold on
> the anon_vma needed to guide both lookups); but I'll have made it
> more glaringly wrong with the VM_BUG_ON_PAGE() - let me pretend
> that's a good step forward :)
>
> There's a reference count check in isolated_migratepages_block()
> before this, which would make it unlikely, but I doubt rules it out.
>
> However... you did hit an anon_vma reference counting problem before
> my migration changes went in, and Kirill had a vague suspicion that
> he might be screwing up anon_vma refcounting in split_huge_page():
> if he confirms that, I'd say it's more likely to be the cause of
> your crash on this occasion.
>
> Not hard to fix mine (though we'll probably have to lose the
> VM_BUG_ON_PAGE on the way, so the real fix will be hidden by that
> trivial fix), I just want to give the races more thought.
And after giving it more thought, I realize that I was wrong yesterday,
and the new VM_BUG_ON_PAGE() should be good as is: my guess is that it
is simply alerting you to the same anon_vma reference counting issue
as you had already hit without that patch.
What I was forgetting yesterday, is that isolate_migratepages_block()
can only take the page for migration when it's PageLRU(): and
do_anonymous_page() only adds a page to the LRU after it has been
marked as mapped and PageAnon.
So the window that worried me yesterday, that __unmap_and_move()
might see !PageAnon, then reach try_to_unmap() with it page_mapped
and PageAnon: that window does not exist, with or without my changes.
Hugh
>
> However it turns out, I think you have a very useful test there.
>
> (And I've observed no PageDirty problems with your recent patchsets,
> though I don't use MADV_FREE at all myself.)
>
> Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists