On Fri, Mar 18, 2022 at 11:04:29AM -0700, Yang Shi wrote:
> I agree once page cache huge page is fully supported,
> READ_ONLY_THP_FOR_FS could be deprecated. But actually this patchset
> makes khugepaged collapse file THP more consistently. It guarantees
> the THP could be collapsed as long as file THP is supported and
> configured properly and there is suitable file vmas, it is not
> guaranteed by the current code. So it should be useful even though
> READ_ONLY_THP_FOR_FS is gone IMHO.
I don't know if it's a good thing or not. Experiments with 64k
PAGE_SIZE on arm64 shows some benchmarks improving and others regressing.
Just because we _can_ collapse a 2MB range of pages into a single 2MB
page doesn't mean we _should_. I suspect the right size folio for any
given file will depend on the access pattern. For example, dirtying a
few bytes in a folio will result in the entire folio being written back.
Is that what you want? Maybe! It may prompt the filesystem to defragment
that range, which would be good. On the other hand, if you're bandwidth
limited, it may decrease your performance. And if your media has limited
write endurance, it may result in your drive wearing out more quickly.
Changing the heuristics should come with data. Preferably from a wide
range of systems and use cases. I know that's hard to do, but how else
can we proceed?
And I think you ignored my point that READ_ONLY_THP_FOR_FS required
no changes to filesystems. It was completely invisible to them, by
design. Now this patchset requires each filesystem to do something.
That's not a great step.
P.S. khugepaged currently does nothing if a range contains a compound
page. It assumes that the page is compound because it's now a THP.
Large folios break that assumption, so khugepaged will now never
collapse a range which includes large folios. Thanks to commit
mm/filemap: Support VM_HUGEPAGE for file mappings
we'll always try to bring in PMD-sized pages for MADV_HUGEPAGE, so
it _probably_ doesn't matter. But it's something we should watch
for as filesystems grow support for large folios.
