On Sun, Mar 10, 2024 at 08:23:12AM +0000, Ryan Roberts wrote:
> It doesn't sound completely impossible to me that there is a rare error path that accidentally folio_put()s an extra time...
Your debug below seems to prove that it's an extra folio_put()
somewhere.
> > list_for_each_entry_safe(folio, next, &ds_queue->split_queue,
> > _deferred_list) {
> > + VM_BUG_ON_FOLIO(folio_nid(folio) != sc->nid, folio);
> > + VM_BUG_ON_FOLIO(folio_order(folio) < 2, folio);
> > list_del_init(&folio->_deferred_list);
> >
> > (also testing the hypothesis that somehow a split folio has ended up
> > on the deferred split list)
>
> OK, ran with these checks, and get the following oops:
>
> [ 411.719461] page:0000000059c1826b refcount:0 mapcount:0 mapping:0000000000000000 index:0x1 pfn:0x8c6a40
> [ 411.720807] page:0000000059c1826b refcount:0 mapcount:-128 mapping:0000000000000000 index:0x1 pfn:0x8c6a40
> [ 411.721792] flags: 0xbfffc0000000000(node=0|zone=2|lastcpupid=0xffff)
> [ 411.722453] page_type: 0xffffff7f(buddy)
> [ 411.722870] raw: 0bfffc0000000000 fffffc001227e808 fffffc002a857408 0000000000000000
> [ 411.723672] raw: 0000000000000001 0000000000000004 00000000ffffff7f 0000000000000000
> [ 411.724470] page dumped because: VM_BUG_ON_FOLIO(!folio_test_large(folio))
> [ 411.725176] ------------[ cut here ]------------
> [ 411.725642] kernel BUG at include/linux/mm.h:1191!
> [ 411.726341] Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
> [ 411.727021] Modules linked in:
> [ 411.727329] CPU: 40 PID: 2704 Comm: usemem Not tainted 6.8.0-rc5-00391-g44b0dc848590-dirty #45
> [ 411.728179] Hardware name: linux,dummy-virt (DT)
> [ 411.728657] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 411.729381] pc : __dump_page+0x450/0x4a8
> [ 411.729789] lr : __dump_page+0x450/0x4a8
> [ 411.730187] sp : ffff80008b97b6f0
> [ 411.730525] x29: ffff80008b97b6f0 x28: 00000000000000e2 x27: ffff80008b97b988
> [ 411.731227] x26: ffff80008b97b988 x25: ffff800082105000 x24: 0000000000000001
> [ 411.731926] x23: 0000000000000000 x22: 0000000000000001 x21: fffffc00221a9000
> [ 411.732630] x20: fffffc00221a9000 x19: fffffc00221a9000 x18: ffffffffffffffff
> [ 411.733331] x17: 3030303030303030 x16: 2066376666666666 x15: 076c076f07660721
> [ 411.734035] x14: 0728074f0749074c x13: 076c076f07660721 x12: 0000000000000000
> [ 411.734757] x11: 0720072007200729 x10: ffff0013f5e756c0 x9 : ffff80008014b604
> [ 411.735473] x8 : 00000000ffffbfff x7 : ffff0013f5e756c0 x6 : 0000000000000000
> [ 411.736198] x5 : ffff0013a5a24d88 x4 : 0000000000000000 x3 : 0000000000000000
> [ 411.736923] x2 : 0000000000000000 x1 : ffff0000c2849b80 x0 : 000000000000003e
> [ 411.737621] Call trace:
> [ 411.737870] __dump_page+0x450/0x4a8
> [ 411.738229] dump_page+0x2c/0x70
> [ 411.738551] deferred_split_scan+0x258/0x368
> [ 411.738973] do_shrink_slab+0x184/0x750
>
> The new VM_BUG_ON_FOLIO(folio_order(folio) < 2, folio); is firing, but then when dump_page() does this:
>
> if (compound) {
> pr_warn("head:%p order:%u entire_mapcount:%d nr_pages_mapped:%d pincount:%d\n",
> head, compound_order(head),
> folio_entire_mapcount(folio),
> folio_nr_pages_mapped(folio),
> atomic_read(&folio->_pincount));
> }
>
> VM_BUG_ON_FOLIO(!folio_test_large(folio), folio); inside folio_entire_mapcount() fires so we have a nested oops.
Ah. I'm not sure what 44b0dc848590 is -- probably a local commit, but
I guess you don't have fae7d834c43c in it which would prevent the nested
oops. Nevertheless, the nested oops does tell us something interesting.
> So the very first line is from the first oops and the rest is from the second. I guess we are racing with the page being freed? I find the change in mapcount interesting; 0 -> -128. Not sure why this would happen?
That's PG_buddy being set in PageType.
> Given the NID check didn't fire, I wonder if this points more towards extra folio_put than corrupt folio nid?
Must be if PG_buddy got set. But we're still left with the question of
how the page gets freed while still being on the deferred list and
doesn't trigger bad_page(page, "still on deferred list") ...
Anyway, we've made some progress. We now understand how a freed page
gets its deferred list overwritten -- we've found a split page on the
deferred list with refcount 0, we _assumed_ it was still intact and
overwrote a different page's ->mapping. And it makes sense that my
patch opened the window wider to hit this problem.
I just checked that free_unref_folios() still does the right thing, and
that also relies on the page not yet being split:
unsigned int order = folio_order(folio);
if (order > 0 && folio_test_large_rmappable(folio))
folio_undo_large_rmappable(folio);
if (!free_unref_page_prepare(&folio->page, pfn, order))
continue;
so there shouldn't be a point in the page freeing process where the
folio is split before we take it off the deferred list.
split_huge_page_to_list_to_order() is also very careful to take the
ds_queue->split_queue_lock before freezing the folio ref, so it's
not a race with that. I don't see what it is yet.
