On Thu, Aug 21, 2025 at 10:55 AM Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> wrote: > > On Thu, Aug 21, 2025 at 10:46:18AM -0600, Nico Pache wrote: > > > > > > Thanks and I"ll have a look, but this series is unmergeable with a broken > > > > > > default in > > > > > > /sys/kernel/mm/transparent_hugepage/khugepaged/mthp_max_ptes_none_ratio > > > > > > sorry. > > > > > > > > > > > > We need to have a new tunable as far as I can tell. I also find the use of > > > > > > this PMD-specific value as an arbitrary way of expressing a ratio pretty > > > > > > gross. > > > > > The first thing that comes to mind is that we can pin max_ptes_none to > > > > > 255 if it exceeds 255. It's worth noting that the issue occurs only > > > > > for adjacently enabled mTHP sizes. > > > > > > No! Presumably the default of 511 (for PMDs with 512 entries) is set for a > > > reason, arbitrarily changing this to suit a specific case seems crazy no? > > We wouldn't be changing it for PMD collapse, just for the new > > behavior. At 511, no mTHP collapses would ever occur anyways, unless > > you have 2MB disabled and other mTHP sizes enabled. Technically at 511 > > only the highest enabled order always gets collapsed. > > > > Ive also argued in the past that 511 is a terrible default for > > anything other than thp.enabled=always, but that's a whole other can > > of worms we dont need to discuss now. > > > > with this cap of 255, the PMD scan/collapse would work as intended, > > then in mTHP collapses we would never introduce this undesired > > behavior. We've discussed before that this would be a hard problem to > > solve without introducing some expensive way of tracking what has > > already been through a collapse, and that doesnt even consider what > > happens if things change or are unmapped, and rescanning that section > > would be helpful. So having a strictly enforced limit of 255 actually > > seems like a good idea to me, as it completely avoids the undesired > > behavior and does not require the admins to be aware of such an issue. > > > > Another thought similar to what (IIRC) Dev has mentioned before, if we > > have max_ptes_none > 255 then we only consider collapses to the > > largest enabled order, that way no creep to the largest enabled order > > would occur in the first place, and we would get there straight away. > > > > To me one of these two solutions seem sane in the context of what we > > are dealing with. > > > > > > > > > > > > > ie) > > > > > if order!=HPAGE_PMD_ORDER && khugepaged_max_ptes_none > 255 > > > > > temp_max_ptes_none = 255; > > > > Oh and my second point, introducing a new tunable to control mTHP > > > > collapse may become exceedingly complex from a tuning and code > > > > management standpoint. > > > > > > Umm right now you hve a ratio expressed in PTES per mTHP * ((PTEs per PMD) / > > > PMD) 'except please don't set to the usual default when using mTHP' and it's > > > currently default-broken. > > > > > > I'm really not sure how that is simpler than a seprate tunable that can be > > > expressed as a ratio (e.g. percentage) that actually makes some kind of sense? > > I agree that the current tunable wasn't designed for this, but we > > tried to come up with something that leverages the tunable we have to > > avoid new tunables and added complexity. > > > > > > And we can make anything workable from a code management point of view by > > > refactoring/developing appropriately. > > What happens if max_ptes_none = 0 and the ratio is 50% - 1 pte > > (ideally the max number)? seems like we would be saying we want no new > > none pages, but also to allow new none pages. To me that seems equally > > broken and more confusing than just taking a scale of the current > > number (now with a cap). > > > > > > The one thing we absolutely cannot have is a default that causes this > 'creeping' behaviour. This feels like shipping something that is broken and > alluding to it in the documentation. Ok I've put a lot of thought and time into this and came up with a solution. Here is what I currently have tested and would like to proposing: - Expand bitmap to HPAGE_PMD_NR (512)*, this increases the accuracy of the max_pte_none handling, and removes a lot of inaccuracies caused by the compression into 128 bits that was being done. This also makes the code a lot easier to understand. - When attempting mTHP level collapses cap max_ptes_none to 255 to prevent the creep issue Ive tested this and found this performs better than my previous version, allows for more granular control via max_ptes_none, and prevents the creep issue without any admin knowledge needed. I think this is a good middle ground between completely disabling the fine tune control, and doing a better job at mitigating misconfiguration. **Baolin actually also expands the bitmap to 512 in his khugepaged collapse file mTHP support patchset Does this sound reasonable to you? -- Nico > > I spoke to David off-list and he gave some insight into this and perhaps > some reasonable means of avoiding an additional tunable. > > I don't want to rehash what he said as I think it's more productive for him > to reply when he has time but broadly I think how we handle this needs > careful consideration. > > To me it's clear that some sense of ratio is just immediately very very > confusing, but then again this interface is already confusing, as with much > of THP. > > Anyway I'll let David respond here so we don't loop around before he has a > chance to add his input. > > Cheers, Lorenzo >
