Update mlock_folio(), munlock_folio() and their fbatch callouts and helpers, to do folio_try_get()s at batch processing time, instead of holding a folio reference all the while in mlock_fbatch: as in folio.c. But more interesting is the use of mod_mlock_count(), using try_cmpxchg() to update folio->mlock_count safely when possible (now when on lru_add fbatch as well as when unevictable). While __mlock_folio() is as hard to think about as before, __munlock_folio() simpler because munlock_folio() can adjust mlock_count itself without clear_lru() or lruvec lock, and so do the folio_test_clear_mlocked() immediately for itself (without which unevictable_pgs_cleared was likely to appear high, when it should be 0 or low to indicate good mlock health). __munlock_folio() is safe for use even when the unreferenced folio has been freed and reused. It appears that __mlock_folio() could affect a folio which has been freed and reused, but only if it is reused as an mlocked folio, in which case its mlock_count is spuriously incremented (but usually a spurious munlock decrement will follow). How grave is this? If unevictable_pgs_cleared remains low, not so bad. I've gone back and forth on whether to move mlock_fbatch and these functions into mm/folio.c: for now they stay here in mm/mlock.c. Signed-off-by: Hugh Dickins --- mm/mlock.c | 147 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 86 insertions(+), 61 deletions(-) diff --git a/mm/mlock.c b/mm/mlock.c index 53d754e82ba2..1050010bbe0b 100644 --- a/mm/mlock.c +++ b/mm/mlock.c @@ -58,6 +58,20 @@ EXPORT_SYMBOL(can_do_mlock); * indicate the unevictable state. */ +static long mod_mlock_count(struct folio *folio, long incdec) +{ + long mlock_count = READ_ONCE(folio->mlock_count); + + while (mlock_count & MLOCK_COUNT_0) { + if (mlock_count + incdec < MLOCK_COUNT_0) + return MLOCK_COUNT_0; + if (try_cmpxchg(&folio->mlock_count, &mlock_count, + mlock_count + incdec)) + return mlock_count + incdec; + } + return 0; +} + static struct lruvec *__mlock_folio(struct folio *folio, struct lruvec *lruvec) { /* There is nothing more we can do while it's off LRU */ @@ -65,6 +79,7 @@ static struct lruvec *__mlock_folio(struct folio *folio, struct lruvec *lruvec) return lruvec; lruvec = folio_lruvec_relock_irq(folio, lruvec); + lruvec_del_folio(lruvec, folio); if (unlikely(folio_evictable(folio))) { /* @@ -73,92 +88,82 @@ static struct lruvec *__mlock_folio(struct folio *folio, struct lruvec *lruvec) * folio be unevictable? I'm not sure, but move it now if so. */ if (folio_test_unevictable(folio)) { - lruvec_del_folio(lruvec, folio); folio_clear_unevictable(folio); - lruvec_add_folio(lruvec, folio); - __count_vm_events(UNEVICTABLE_PGRESCUED, folio_nr_pages(folio)); } goto out; } + /* + * Something to keep in mind when studying the arithmetic here: + * we only come to __mlock_folio() when mlock_folio() could not + * mod_mlock_count() itself; but by the time this is processed, + * the folio may have already been munlocked, or another mlock + * already marked it as unevictable and so mod_mlock_countable. + * And don't forget that a folio may be unevictable for reasons + * other than mlocked (hence the folio_evictable() check above). + */ + if (folio_test_unevictable(folio)) { if (folio_test_mlocked(folio)) - folio->mlock_count += MLOCK_COUNT_1; + mod_mlock_count(folio, MLOCK_COUNT_1); goto out; } - lruvec_del_folio(lruvec, folio); folio_clear_active(folio); folio_set_unevictable(folio); - folio->mlock_count = MLOCK_COUNT_0; - if (folio_test_mlocked(folio)) - folio->mlock_count += MLOCK_COUNT_1; - lruvec_add_folio(lruvec, folio); __count_vm_events(UNEVICTABLE_PGCULLED, folio_nr_pages(folio)); + + if (!folio_test_mlocked(folio)) + folio->mlock_count = MLOCK_COUNT_0; + else if (!mod_mlock_count(folio, MLOCK_COUNT_1)) + folio->mlock_count = MLOCK_COUNT_0 + MLOCK_COUNT_1; out: + lruvec_add_folio(lruvec, folio); folio_set_lru(folio); return lruvec; } static struct lruvec *__munlock_folio(struct folio *folio, struct lruvec *lruvec) { - int nr_pages = folio_nr_pages(folio); - bool isolated = false; + long nr_pages = folio_nr_pages(folio); - if (!folio_test_clear_lru(folio)) - goto munlock; - - isolated = true; - lruvec = folio_lruvec_relock_irq(folio, lruvec); - - if (folio_test_unevictable(folio)) { - /* Then mlock_count is maintained, but might undercount */ - if (folio->mlock_count > MLOCK_COUNT_0) - folio->mlock_count -= MLOCK_COUNT_1; - if (folio->mlock_count > MLOCK_COUNT_0) - goto out; - } - /* else assume that was the last mlock: reclaim will fix it if not */ - -munlock: - if (folio_test_clear_mlocked(folio)) { - __zone_stat_mod_folio(folio, NR_MLOCK, -nr_pages); - if (isolated || !folio_test_unevictable(folio)) - __count_vm_events(UNEVICTABLE_PGMUNLOCKED, nr_pages); - else + /* There is nothing more we can do while it's off LRU */ + if (!folio_test_clear_lru(folio)) { + if (folio_test_unevictable(folio) && folio_evictable(folio)) __count_vm_events(UNEVICTABLE_PGSTRANDED, nr_pages); + /* But whoever puts it back on LRU should rescue it */ + return lruvec; } - /* folio_evictable() has to be checked *after* clearing Mlocked */ - if (isolated && folio_test_unevictable(folio) && folio_evictable(folio)) { - lruvec_del_folio(lruvec, folio); + lruvec = folio_lruvec_relock_irq(folio, lruvec); + lruvec_del_folio(lruvec, folio); + + if (folio_test_unevictable(folio) && folio_evictable(folio)) { folio_clear_unevictable(folio); - lruvec_add_folio(lruvec, folio); __count_vm_events(UNEVICTABLE_PGRESCUED, nr_pages); } -out: - if (isolated) - folio_set_lru(folio); + + lruvec_add_folio(lruvec, folio); + folio_set_lru(folio); return lruvec; } /* - * Flags held in the low bits of a struct folio pointer on the mlock_fbatch. + * Flag held in the low bits of a struct folio pointer on the mlock_fbatch. */ -#define LRU_FOLIO 0x1 -static inline struct folio *mlock_lru(struct folio *folio) +#define MLOCK_FLAG 0x1 +static inline struct folio *mlock_flagged(struct folio *folio) { - return (struct folio *)((unsigned long)folio + LRU_FOLIO); + return (struct folio *)((unsigned long)folio + MLOCK_FLAG); } /* * mlock_folio_batch() is derived from folio_batch_move_lru(): perhaps that can * make use of such folio pointer flags in future, but for now just keep it for - * mlock. We could use three separate folio batches instead, but one feels - * better (munlocking a full folio batch does not need to drain mlocking folio - * batches first). + * mlock. We could use separate folio batches instead, but one feels better + * (munlocking a full folio batch does not need to drain mlocking batch first). */ static void mlock_folio_batch(struct folio_batch *fbatch) { @@ -169,10 +174,15 @@ static void mlock_folio_batch(struct folio_batch *fbatch) for (i = 0; i < folio_batch_count(fbatch); i++) { folio = fbatch->folios[i]; - mlock = (unsigned long)folio & LRU_FOLIO; + mlock = (unsigned long)folio & MLOCK_FLAG; folio = (struct folio *)((unsigned long)folio - mlock); fbatch->folios[i] = folio; + if (!folio_try_get(folio)) { + fbatch->folios[i] = NULL; + continue; + } + if (mlock) lruvec = __mlock_folio(folio, lruvec); else @@ -218,19 +228,25 @@ void mlock_folio(struct folio *folio) { struct folio_batch *fbatch; - local_lock(&mlock_fbatch.lock); - fbatch = this_cpu_ptr(&mlock_fbatch.fbatch); - if (!folio_test_set_mlocked(folio)) { - int nr_pages = folio_nr_pages(folio); + long nr_pages = folio_nr_pages(folio); zone_stat_mod_folio(folio, NR_MLOCK, nr_pages); - __count_vm_events(UNEVICTABLE_PGMLOCKED, nr_pages); + count_vm_events(UNEVICTABLE_PGMLOCKED, nr_pages); } - folio_get(folio); - if (!folio_batch_add(fbatch, mlock_lru(folio)) || - true || /* XXX Temporarily disable mlock batching */ + /* + * No more to do if mlock_count is maintained: either the folio + * is on an lru_add fbatch, and will be moved to unevictable in + * due course, or it's already counted as unevictable: no need + * for an mlock_fbatch entry below. + */ + if (mod_mlock_count(folio, MLOCK_COUNT_1)) + return; + + local_lock(&mlock_fbatch.lock); + fbatch = this_cpu_ptr(&mlock_fbatch.fbatch); + if (!folio_batch_add(fbatch, mlock_flagged(folio)) || !folio_may_be_lru_cached(folio) || lru_cache_disabled()) mlock_folio_batch(fbatch); local_unlock(&mlock_fbatch.lock); @@ -244,15 +260,24 @@ void munlock_folio(struct folio *folio) { struct folio_batch *fbatch; + /* + * No more to do if mlock_count is maintained and still raised. + * But if mlock_count is unmaintained, we might need to queue an + * munlock fbatch entry, just to cancel an undequeued mlock entry? + */ + if (mod_mlock_count(folio, -MLOCK_COUNT_1) > MLOCK_COUNT_0) + return; + + if (folio_test_clear_mlocked(folio)) { + long nr_pages = folio_nr_pages(folio); + + zone_stat_mod_folio(folio, NR_MLOCK, -nr_pages); + count_vm_events(UNEVICTABLE_PGMUNLOCKED, nr_pages); + } + local_lock(&mlock_fbatch.lock); fbatch = this_cpu_ptr(&mlock_fbatch.fbatch); - /* - * folio_test_clear_mlocked(folio) must be left to __munlock_folio(), - * which will check whether the folio is multiply mlocked. - */ - folio_get(folio); if (!folio_batch_add(fbatch, folio) || - true || /* XXX Temporarily disable munlock batching */ !folio_may_be_lru_cached(folio) || lru_cache_disabled()) mlock_folio_batch(fbatch); local_unlock(&mlock_fbatch.lock); -- 2.51.0