On Thu, Aug 17, 2023 at 12:01 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > On Thu, Aug 17, 2023 at 11:13:36AM -0700, Zach O'Keefe wrote: > > > > IIUC then, there is a bug in smaps THPeligible code when > > > > CONFIG_READ_ONLY_THP_FOR_FS is not set. Not obvious, but apparently > > > > this config is (according to it's Kconfig desc) khugepaged-only, so it > > > > should be fine for it to be disabled, yet allow > > > > do_sync_mmap_readahead() to install a pmd for file-backed memory. > > > > hugepage_vma_check() will need to be patched to fix this. > > > > > > I guess so ... > > > > The easiest and most satisfying way to handle this -- and I think we > > talked about this before -- is relaxing that complicated > > file_thp_enabled() check when the file's mapping supports large > > folios. I think that makes sense to me, though I don't know all the > > details fs-side. Will we need any hook to give fs the chance to update > > any internal state on collapse? > > If the filesystem has per-folio metadata, we need to give the filesystem > the chance to set that up. I've vaguaely been wondering about using the > ->migrate_folio callback for it. At the moment, I think it just refuses > to work if the folio isn't order-0. Ok, no free lunch here then. I'll give myself a reminder to come back here then and dig a little deeper. Thanks Matthew > > > > But I have a larger question for you: should we care about > > > > /sys/kernel/mm/transparent_hugepage/enabled for file-fault? We > > > > currently don't. Seems weird that we can transparently get a hugepage > > > > when THP="never". Also, if THP="always", we might as well skip the > > > > VM_HUGEPAGE check, and try the final pmd install (and save khugepaged > > > > the trouble of attempting it later). > > > > > > I deliberately ignored the humungous complexity of the THP options. > > > They're overgrown and make my brain hurt. [..] > > > > Same > > > > > [..] Instead, large folios are > > > adaptive; they observe the behaviour of the user program and choose based > > > on history what to do. This is far superior to having a sysadmin tell > > > us what to do! > > > > I had written a bunch on this, but I arrived to the conclusion that > > (a) pmd-mapping here is ~ a free win, and (b) I'm not the best person > > to argue for these knobs, given MADV_COLLAPSE ignores them entirely :P > > > > ..But (sorry) what about MMF_DISABLE_THP? > > Yeah, we ignore that too. My rationale is -- as you said -- using the > PMDs is actually free, and it's really none of the app's business how > the page cache chooses to cache things. What should be done to be consistent with the collapse side here, for file/shmem if at all? Answering the question, "can this memory be backed by THPs" is becoming really complex, and that THPelligble smaps field is becoming increasingly more difficult to use.
