Re: [PATCH] mm/mlock: use the IRQ-safe accessor for NR_MLOCK in __munlock_folio()

[Hugh Dickins <hughd@google.com>]

On Tue, 1 Sep 2026, Shakeel Butt wrote:

> NR_MLOCK is updated from interrupt context.  __free_pages_prepare() clears
> a stray PG_mlocked and adjusts NR_MLOCK, and a folio can reach it with the
> flag still set from a bio completion handler:
> 
>   __free_pages_ok+0x6af/0x7a0
>   <IRQ>
>   __bio_release_pages+0xde/0x260
>   __iomap_dio_bio_end_io+0x16e/0x1a0
>   blk_update_request+0x14b/0x3d0
>   blk_mq_end_request+0x18/0x30
>   blk_done_softirq+0x49/0x60
> 
> The folio gets there like this.  A MAP_SHARED file mapping is mlocked, so
> its page cache folios carry PG_mlocked, and an O_DIRECT write sourced from
> that mapping GUP-pins those same folios.  munlock() then runs
> mlock_vma_pages_range(), which clears VM_LOCKED before walking the page
> tables to munlock each folio.  A concurrent hole punch reaches the folio
> through the rmap (i_mmap_rwsem, not mmap_lock) and can land inside that
> window: __folio_remove_rmap() -> munlock_vma_folio() sees VM_LOCKED
> already clear, so it neither queues the folio on the mlock batch nor takes
> a reference, and the pte it clears makes the pending mlock_pte_range()
> walk skip the folio at its !pte_present() check.  filemap_remove_folio()
> then drops the page cache reference, leaving the bio's pin as the last
> one, released from the completion handler above.

I'm hardly ashamed to admit that I've not tried to digest
that paragraph. You're writing about the rare fallback cases when
PG_mlocked is cleared late, and unevictable_pgs_cleared incremented to
notify us of that defect: yes, I accept that might happen at interrupt
time, and so we ought not to take the __shortcut in __munlock_folio()
which you fix  below.

> 
> So __zone_stat_mod_folio() here needs interrupts disabled, not merely
> preemption, and __munlock_folio() has a path where they are not: when the
> folio has already been taken off the LRU by somebody else the function
> jumps straight to the counter update without taking the lruvec lock.  The
> read-modify-write of the per-CPU NR_MLOCK diff can then be interrupted by
> the softirq above, and one of the two decrements is lost, leaving Mlocked
> in /proc/meminfo permanently overstated.
> 
> Use zone_stat_mod_folio().  mod_zone_state()'s this_cpu_try_cmpxchg() is
> atomic against a same-CPU interrupt and retries, and on the path where the
> lruvec lock is held its cost is negligible next to the lock itself.
> 
> The UNEVICTABLE_PG* events are deliberately left on the __ accessors:
> they occupy different vm_event_states slots from the UNEVICTABLE_PGCLEARED
> that __free_pages_prepare() bumps, and nothing updates those two from
> interrupt context.
> 
> Fixes: 2fbb0c10d1e8 ("mm/munlock: mlock_page() munlock_page() batch by pagevec")
> Cc: <stable@vger.kernel.org>

Okay: just a wrong stat, but it ought to go back to 0, so Cc stable yes.

> Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>

Acked-by: Hugh Dickins <hughd@google.com>

But I do think you (or Andrew :-) should include

Reported-by: syzbot+cd2073ee6d958a8d0fcd@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/linux-mm/6a931c5a.08e933ee.dbf97.0093.GAE@google.com/

That was indeed reporting a different way to get a WARNING from this,
when offlining a CPU: but it should be acknowledged for bringing you
here, and we should tell syzbot it's fixed by this.

I've been trying to work out whether you're going to come back in a
day or two, changing the __count_vm_events() too: and had raised in
that thread the question of why lru_add_drain()'s __count_vm_events
were not also reported by syzbot; but now I can see
 * vm counters are allowed to be racy. Use raw_cpu_ops to avoid the
 * local_irq_disable overhead.
and realize that they're not a problem; so this looks complete, we
shouldn't need local_lock()ing in mlock_drain_remote() after all.

Thanks,
Hugh

> ---
>  mm/mlock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/mlock.c b/mm/mlock.c
> index efa6716e4dfb..39215a3eab1f 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -141,7 +141,7 @@ static struct lruvec *__munlock_folio(struct folio *folio, struct lruvec *lruvec
>  
>  munlock:
>  	if (folio_test_clear_mlocked(folio)) {
> -		__zone_stat_mod_folio(folio, NR_MLOCK, -nr_pages);
> +		zone_stat_mod_folio(folio, NR_MLOCK, -nr_pages);
>  		if (isolated || !folio_test_unevictable(folio))
>  			__count_vm_events(UNEVICTABLE_PGMUNLOCKED, nr_pages);
>  		else
> -- 
> 2.53.0-Meta