Thread (119 messages) flat view 119 messages, 9 authors, 11d ago

Re: [RFC PATCH 16/57] mm/collapse: freeze the sources behind migration entries

From: Usama Arif <usama.arif@linux.dev>
Date: 2026-08-24 16:13:55
Also in: bpf, linux-kselftest, linux-mm, lkml


On 24/08/2026 16:49, Johannes Weiner wrote:
On Mon, Aug 24, 2026 at 03:13:10PM +0100, Kiryl Shutsemau wrote:
quoted
On Mon, Aug 24, 2026 at 09:12:24PM +0800, Lance Yang wrote:
quoted
On Sun, Aug 16, 2026 at 11:45:28PM +0100, Kiryl Shutsemau wrote:
quoted
+		if (!folio_ref_freeze(folio,
+				      folio_expected_ref_count(folio) + 1)) {
+			result = SCAN_PAGE_COUNT;
+			goto unfreeze;
+		}
+		nr_frozen = nr_saved;
Just one thing I was wondering about ... can deferred_split_isolate()
remove a source folio from deferred_split_lru while its refcount is
frozen by collapse_freeze_candidate()?

Assume an earlier span belongs to an anonymous large folio on the
deferred split queue, then a later span fails folio_trylock().
collapse_freeze_candidate() continues after freezing each source folio
and calls collapse_unfreeze_candidate() on a later failure:

static noinline enum scan_result collapse_freeze_candidate(struct mm_struct *mm,
		struct collapse_candidate *cand, pte_t *pte)
{
...
	for (i = 0, addr = cand->addr; i < nr_pages;) {
...
		if (!folio_trylock(folio)) {
			folio_put(folio);
			result = SCAN_PAGE_LOCK;
			goto unfreeze;
		}
...
		nr_saved = i + nr;

		if (!folio_ref_freeze(folio,
				      folio_expected_ref_count(folio) + 1)) {
			result = SCAN_PAGE_COUNT;
			goto unfreeze;
		}
		nr_frozen = nr_saved;

		i += nr;
		addr += nr * PAGE_SIZE;
	}

...
unfreeze:
	collapse_unfreeze_candidate(mm, cand, pte, nr_saved, nr_frozen);
	return result;
}

folio_ref_freeze() takes the source folio's refcount to zero:

static inline int folio_ref_freeze(struct folio *folio, int count)
{
	return page_ref_freeze(&folio->page, count);
}

static inline int page_ref_freeze(struct page *page, int count)
{
	int ret = likely(atomic_cmpxchg(&page->_refcount, count, 0) == count);

...
	return ret;
}

While collapse_freeze_candidate() still holds the source folio lock,
deferred_split_scan() can call deferred_split_isolate():

static unsigned long deferred_split_scan(struct shrinker *shrink,
		struct shrink_control *sc)
{
	LIST_HEAD(dispose);
	struct folio *folio, *next;
	int split = 0;
	unsigned long isolated;

	isolated = list_lru_shrink_walk_irq(&deferred_split_lru, sc,
					    deferred_split_isolate, &dispose);
}

static enum lru_status deferred_split_isolate(struct list_head *item,
					      struct list_lru_one *lru,
					      void *cb_arg)
{
	struct folio *folio = container_of(item, struct folio, _deferred_list);
	struct list_head *freeable = cb_arg;

	if (folio_try_get(folio)) {
		list_lru_isolate_move(lru, item, freeable);
		return LRU_REMOVED;
	}

	/*
	 * We lost race with folio_put(). Read folio state before the
	 * isolate: folio_unqueue_deferred_split() checks list_empty()
	 * locklessly, so once removed the folio can be freed any time.
	 */
	if (folio_test_partially_mapped(folio)) {
		folio_clear_partially_mapped(folio);
		mod_mthp_stat(folio_order(folio),
			      MTHP_STAT_NR_ANON_PARTIALLY_MAPPED, -1);
	}
	list_lru_isolate(lru, item);
	return LRU_REMOVED;
}

And folio_try_get() fails because the source folio has a frozen refcount.
deferred_split_isolate() treats the failure as a race with folio_put(),
clears PG_partially_mapped and its MTHP_STAT_NR_ANON_PARTIALLY_MAPPED
accounting when set, then removes the folio from deferred_split_lru ...
Hm. So, the premise in deferred_split_isolate() is false:
!folio_try_get() doesn't mean lost race with folio_put().
I suppose you mean, not exclusively. But it can also mean that. And
then the question is, who cleans up the partially_mapped state.
quoted
I think deferred_split_isolate() should do something like:

      if (!folio_try_get(folio))
              return LRU_SKIP;

      list_lru_isolate_move(lru, item, freeable);
      return LRU_REMOVED;

Johannes, do I miss something?
Ah. The idea being: leave the item on the LRU when there is a race
with the refcount going zero; and then it's up to that other side to
deal with/clean up the partially_mapped state as appropriate. Thus:

folio_put()
  __folio_put()
    folio_unqueue_deferred_split()
      if __list_lru_del():
        // clear partially_mapped state & stats

will always succeed, even if it races with the shrinker. And you are
also guaranteed on the collapse side that the state won't vanish from
underneath you.

I think that should work. Usama?

Kiryl's suggestion makes sense. I think there is a bug here. If we dont
get the reference, whoever owns the reference should decide how partially_mapped
is treated for that folio:

- If its the last folio_put(), it will clear partially_mapped and cleanup
after itself.
- If its folio_ref_freeze(), clearing partially_mapped is wrong (which we
are currently doing in deferred_split_isolate)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help