On Sun, Mar 10, 2024 at 08:46:58PM +0000, Matthew Wilcox wrote:
> On Sun, Mar 10, 2024 at 07:59:46PM +0000, Ryan Roberts wrote:
> > I've now been able to repro this without any of my code on top - just mm-unstable and your fix for the the memcg uncharging ordering issue. So we have separate, more difficultt to repro bug. I've discovered CONFIG_DEBUG_LIST so enabled that. I'll try to bisect in the morning, but I suspect it will be slow going.
> >
> > [ 390.317982] ------------[ cut here ]------------
> > [ 390.318646] list_del corruption. prev->next should be fffffc00152a9090, but was fffffc002798a490. (prev=fffffc002798a490)
>
> Interesting. So prev->next is pointing to prev, ie prev is an empty
> list, but it should be pointing to this entry ... this is feeling like
> another missing lock.
Let's check that we're not inverting the order of memcg_uncharge and
removing a folio from the deferred list (build tested only, but only
one line of this will be new to you):
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index bb57b3d0c8cd..61fd1a4b424d 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -792,8 +792,6 @@ void folio_prep_large_rmappable(struct folio *folio)
{
if (!folio || !folio_test_large(folio))
return;
- if (folio_order(folio) > 1)
- INIT_LIST_HEAD(&folio->_deferred_list);
folio_set_large_rmappable(folio);
}
diff --git a/mm/internal.h b/mm/internal.h
index 79d0848c10a5..690c68c18c23 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -525,6 +525,8 @@ static inline void prep_compound_head(struct page *page, unsigned int order)
atomic_set(&folio->_entire_mapcount, -1);
atomic_set(&folio->_nr_pages_mapped, 0);
atomic_set(&folio->_pincount, 0);
+ if (order > 1)
+ INIT_LIST_HEAD(&folio->_deferred_list);
}
static inline void prep_compound_tail(struct page *head, int tail_idx)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 138bcfa18234..e2334c4ee550 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -7483,6 +7483,8 @@ static void uncharge_folio(struct folio *folio, struct uncharge_gather *ug)
struct obj_cgroup *objcg;
VM_BUG_ON_FOLIO(folio_test_lru(folio), folio);
+ VM_BUG_ON_FOLIO(folio_order(folio) > 1 &&
+ !list_empty(&folio->_deferred_list), folio);
/*
* Nobody should be changing or seriously looking at
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index bdff5c0a7c76..1c1925b92934 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1006,10 +1006,11 @@ static int free_tail_page_prepare(struct page *head_page, struct page *page)
}
break;
case 2:
- /*
- * the second tail page: ->mapping is
- * deferred_list.next -- ignore value.
- */
+ /* the second tail page: deferred_list overlaps ->mapping */
+ if (unlikely(!list_empty(&folio->_deferred_list))) {
+ bad_page(page, "on deferred list");
+ goto out;
+ }
break;
default:
if (page->mapping != TAIL_MAPPING) {
