On Mon, Aug 24, 2026 at 03:13:10PM +0100, Kiryl Shutsemau wrote:
> On Mon, Aug 24, 2026 at 09:12:24PM +0800, Lance Yang wrote:
> >
> > On Sun, Aug 16, 2026 at 11:45:28PM +0100, Kiryl Shutsemau wrote:
> > >+ 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.
> 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?
