Re: [PATCH v3 1/4] mm: swap: Remove CLUSTER_FLAG_HUGE from swap_cluster_info:flags

Matthew Wilcox
2024-03-01T16:31:23Z

On Fri, Mar 01, 2024 at 04:27:32PM +0000, Ryan Roberts wrote:
> I've implemented the batching as David suggested, and I'm pretty confident it's
> correct. The only problem is that during testing I can't provoke the code to
> take the path. I've been pouring through the code but struggling to figure out
> under what situation you would expect the swap entry passed to
> free_swap_and_cache() to still have a cached folio? Does anyone have any idea?
>
> This is the original (unbatched) function, after my change, which caused David's
> concern that we would end up calling __try_to_reclaim_swap() far too much:
>
> int free_swap_and_cache(swp_entry_t entry)
> {
> struct swap_info_struct *p;
> unsigned char count;
>
> if (non_swap_entry(entry))
> return 1;
>
> p = _swap_info_get(entry);
> if (p) {
> count = __swap_entry_free(p, entry);
> if (count == SWAP_HAS_CACHE)
> __try_to_reclaim_swap(p, swp_offset(entry),
> TTRS_UNMAPPED | TTRS_FULL);
> }
> return p != NULL;
> }
>
> The trouble is, whenever its called, count is always 0, so
> __try_to_reclaim_swap() never gets called.
>
> My test case is allocating 1G anon memory, then doing madvise(MADV_PAGEOUT) over
> it. Then doing either a munmap() or madvise(MADV_FREE), both of which cause this
> function to be called for every PTE, but count is always 0 after
> __swap_entry_free() so __try_to_reclaim_swap() is never called. I've tried for
> order-0 as well as PTE- and PMD-mapped 2M THP.

I think you have to page it back in again, then it will have an entry in
the swap cache. Maybe. I know little about anon memory ;-)

If that doesn't work, perhaps use tmpfs, and use some memory pressure to
force that to swap?

> I'm guessing the swapcache was already reclaimed as part of MADV_PAGEOUT? I'm
> using a block ram device as my backing store - I think this does synchronous IO
> so perhaps if I have a real block device with async IO I might have more luck?
> Just a guess...
>
> Or perhaps this code path is a corner case? In which case, perhaps its not worth
> adding the batching optimization after all?
>
> Thanks,
> Ryan
>