Re: [PATCH v2 4/4] mm: try to free swapcache for non-LRU folios

["David Hildenbrand (Arm)" <david@kernel.org>]

On 7/1/26 00:36, Barry Song wrote:
> On Tue, Jun 30, 2026 at 1:48 PM David Hildenbrand (Arm)
> <david@kernel.org> wrote:
> [...]
>>
>> -       if (vmf->flags & FAULT_FLAG_WRITE) {
>> +       if ((vmf->flags & FAULT_FLAG_WRITE) && !pte_write(pte)) {
>>                 ret |= do_wp_page(vmf);
>>                 if (ret & VM_FAULT_ERROR)
>>                         ret &= VM_FAULT_ERROR;
>>
> 
> Thanks! I'm working on a v3, but one thing still seems a bit odd to me.
> 
> In patch 1, we're removing the local LRU drain from
> do_wp_page(). But in patch 4, do_swap_page() calls
> do_wp_page() for folios without the "exclusive" hint,
> since folios with the hint have already been reused in
> do_swap_page().
> 
> This can happen when we allocate a new folio whose swap
> entry lacks the exclusive flag, even though the folio is
> still reusable. For example, in the scenario I mentioned
> in patch 1, the swap count is effectively 1, but there is
> no exclusive flag:
> 
> #include <sys/mman.h>
> 
> int main(int argc, char *argv[])
> {
>     int i;
>     #define SIZE 30*1024*1024
>     while(1) {
>         volatile int *p = mmap(0, SIZE, PROT_READ | PROT_WRITE,
>              MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>         volatile int q;
> 
>         for (int i = 0; i < SIZE/sizeof(int); i++)
>               p[i] =  i;
>         madvise((void *)p, SIZE, MADV_PAGEOUT);
>         if (!fork())
>              _exit(0);
>         for (int i = 0; i < SIZE/sizeof(int); i++)
>               q = p[i];
>         for (int i = 0; i < SIZE/sizeof(int); i++)
>               p[i] =  i;
>         munmap(p, SIZE);
>     }
>     return 0;
> }
> 
> The newly allocated folio will likely still be sitting in this
> CPU's local LRU cache. If we drop the drain in patch 1, it seems
> there is little point in calling do_wp_page() from
> do_swap_page(), since we won't be able to reuse the folio while
> it remains in the local LRU cache.

Well, we call do_wp_page() here to avoid another page fault where we would end
up in do_wp_page() and actually break COW.

> 
> So I'm wondering whether we should keep the drain in
> patch 1 by checking for a refcount of 3, or instead do
> something like the following in patch 4:
> 
> 
>         if ((vmf->flags & FAULT_FLAG_WRITE) && !pte_write(pte)) {
>                  lru_add_drain();
>                  ret |= do_wp_page(vmf);
>                  if (ret & VM_FAULT_ERROR)
>                          ret &= VM_FAULT_ERROR;
>        }
> 
> Otherwise, the combination of patches 1 and 4 seems somewhat
> inconsistent. It feels like a strange mix of sweet, spicy,
> hot, and cold—kind of contradictory :-)

Yes, this needs a proper thought.

What you describe is relevant for

e.g.,

swapout
fork -> exclusive bit cleared
child exit -> parent is single user
swapin -> added to swapcache and LRU cache


This is not very odd, so we should keep that working (I think I have
micro-benchmarks for that).

-- 
Cheers,

David