Hey Lorenzo,
I mean not to beat a dead horse re: v11 commentary, but I thought we were going
to implement David's idea re: the new 'eagerness' tunable, and again we're now
just
implementing the capping at HPAGE_PMD_NR/2 - 1 thing again?
I spoke to David and he said to continue forward with this series; the
"eagerness" tunable will take some time, and may require further
considerations/discussion.
Right, after talking to Johannes it got clearer that what we envisioned with
I'm not sure that you meant to say go ahead with the series as-is with this
silent capping?
No, "go ahead" as in "let's find some way forward that works for all and
is not too crazy".
[...]
"eagerness" would not be like swappiness, and we will really have to be
careful here. I don't know yet when I will have time to look into that.
I guess I missed this part of the converastion, what do you mean?
Johannes raised issues with that on the list and afterwards we had an
offline discussion about some of the details and why something
unpredictable is not good.
The whole concept is that we have a paramaeter whose value is _abstracted_ and
which we control what it means.
I'm not sure exactly why that would now be problematic? The fundamental concept
seems sound no? Last I remember of the conversation this was the case.
The basic idea was to do something abstracted as swappiness. Turns out
"swappiness" is really something predictable, not something we can
randomly change how it behaves under the hood.
So we'd have to find something similar for "eagerness", and that's where
it stops being easy.
If we want to avoid the implicit capping, I think there are the following
possible approaches
(1) Tolerate creep for now, maybe warning if the user configures it.
I mean this seems a viable option if there is pressure to land this series
before we have a viable uAPI for configuring this.
A part of me thinks we shouldn't rush series in for that reason though and
should require that we have a proper control here.
But I guess this approach is the least-worst as it leaves us with the most
options moving forwards.
Yes. There is also the alternative of respecting only 0 / 511 for mTHP
collapse for now as discussed in the other thread.
(2) Avoid creep by counting zero-filled pages towards none_or_zero.
Would this really make all that much difference?
It solves the creep problem I think, but it's a bit nasty IMHO.
(3) Have separate toggles for each THP size. Doesn't quite solve the
problem, only shifts it.
Yeah I did wonder about this as an alternative solution. But of course it then
makes it vague what the parent values means in respect of the individual levels,
unless we have an 'inherit' mode there too (possible).
It's going to be confusing though as max_ptes_none sits at the root khugepaged/
level and I don't think any other parameter from khugepaged/ is exposed at
individual page size levels.
And of course doing this means we
Anything else?
Err... I mean I'm not sure if you missed it but I suggested an approach in the
sub-thread - exposing mthp_max_ptes_none as a _READ-ONLY_ field at:
/sys/kernel/mm/transparent_hugepage/khugepaged/max_mthp_ptes_none
Then we allow the capping, but simply document that we specify what the capped
value will be here for mTHP.
I did not have time to read the details on that so far.
It would be one solution forward. I dislike it because I think the whole
capping is an intermediate thing that can be (and likely must be, when
considering mTHP underused shrinking I think) solved in the future
differently. That's why I would prefer adding this only if there is no
other, simpler, way forward.
That struck me as the simplest way of getting this series landed without
necessarily violating any future eagerness which:
a. Must still support khugepaged/max_ptes_none - we aren't getting away from
this, it's uAPI.
b. Surely must want to do different things for mTHP in eagerness, so if we're
exposing some PTE value in max_ptes_none doing so in
khugepaged/mthp_max_ptes_none wouldn't be problematic (note again - it's
readonly so unlike max_ptes_none we don't have to worry about the other
direction).
HOWEVER, eagerness might want want to change this behaviour per-mTHP size, in
which case perhaps mthp_max_ptes_none would be problematic in that it is some
kind of average.
Then again we could always revert to putting this parameter as in (3) in that
case, ugly but kinda viable.
IIUC, creep is less of a problem when we have the underused shrinker
enabled: whatever we over-allocated can (unless longterm-pinned etc) get
reclaimed again.
So maybe having underused-shrinker support for mTHP as well would be a
solution to tackle (1) later?
How viable is this in the short term?
I once started looking into it, but it will require quite some work,
because the lists will essentially include each and every (m)THP in the
system ... so i think we will need some redesign.
Another possible solution:
If mthp_max_ptes_none is not workable, we could have a toggle at, e.g.:
/sys/kernel/mm/transparent_hugepage/khugepaged/mthp_cap_collapse_none
As a simple boolean. If switched on then we document that it caps mTHP as
per Nico's suggestion.
That way we avoid the 'silent' issue I have with all this and it's an
explicit setting.
Right, but it's another toggle I wish we wouldn't need. We could of
course also make it some compile-time option, but not sure if that's
really any better.
I'd hope we find an easy way forward that doesn't require new toggles,
at least for now ...
--
Cheers
David / dhildenb
