On 17/07/2024 09:02, David Hildenbrand wrote: >>> Sorry, busy with other stuff. >>> >>> Indicating only what really exists sounds cleaner. But I wonder how we would >>> want to handle in general orders that are effectively non-existant? >> >> I'm not following your distinction between orders that don't "really exist" and >> orders that are "effectively non-existant". > > I'm questioning whether there should be a distinction at all. We should just > hide what is either non-existant (not implemented) or non-functional. Great we are on the same page. > >> >> I guess the real supported orders are: >> >> anon: >> min order: 2 >> max order: PMD_ORDER >> anon-shmem: >> min order: 1 >> max order: MAX_PAGECACHE_ORDER >> tmpfs-shmem: >> min order: PMD_ORDER <= 11 ? PMD_ORDER : NONE >> max order: PMD_ORDER <= 11 ? PMD_ORDER : NONE >> file: >> min order: 1 >> max order: MAX_PAGECACHE_ORDER > > That's my understanding. But not sure about anon-shmem really supporting > order-1, maybe we do. Oh, I thought we only had the restriction for anon folios now (due to deferred split queue), so assumed it would just work. With Gavin's THP_ORDERS_ALL_FILE_DEFAULT change, that certainly implies that shmem must support order-1. If it doesn't then we we might want to tidy that further. Baolin, perhaps you can confirm either way? > >> >> But today, controls and stats are exposed for: >> >> anon: >> min order: 2 >> max order: PMD_ORDER >> anon-shmem: >> min order: 2 >> max order: PMD_ORDER >> tmpfs-shmem: >> min order: PMD_ORDER >> max order: PMD_ORDER >> file: >> min order: Nothing yet (this patch proposes 1) >> max order: Nothing yet (this patch proposes MAX_PAGECACHE_ORDER) >> >> So I think there is definitely a bug for shmem where the minimum order control >> should be order-1 but its currently order-2. > > Maybe, did not play with that yet. Likely order-1 will work. (although probably > of questionable use :) ) You might have to expand on why its of "questionable use". I'd assume it has the same amount of value as using order-1 for regular page cache pages? i.e. half the number of objects to manage for the same amount of memory. > >> >> I also wonder about PUD-order for DAX? We don't currently have a stat/control. >> If we wanted to add it in future, if we take the "expose all stats/controls for >> all orders" approach, we would end up extending all the way to PUD-order and all >> the orders between PMD and PUD would be dummy for all memory types. That really >> starts to feel odd, so I still favour only populating what's really supported. > > I would go further and say that calling the fsdax thing a THP is borderline > wrong and we should not expose any new toggles for it that way. > > It really behaves much more like hugetlb folios that can be PTE-mapped ... we > cannot split these things, and they are not allocated from the buddy. So I > wouldn't worry about fsdax for now. > > fsdax support for compound pages (now large folios) probably never should have > been glued to any THP toggle. Yeah fair enough. I wasn't really arguing for adding any dax controls; I was just trying to think of examples as to why adding dummy controls might be a bad idea. > >> >> I propose to fix shmem (extend down to 1, stop at MAX_PAGECACHE_ORDER) and >> continue with the approach of "indicating only what really exists" for v2. >> >> Shout if you disagree. > > Makes sense. Excellent. I posted v2, which has these changes, yesterday afternoon. :)