Re: [PATCH 1/4] mm/swap, memcg: Introduce infrastructure for cgroup-based swap priority

[YoungJun Park <youngjun.park@lge.com>]

> How do you express the default tier who shall not name? There are
> actually 3 states associated with default. It is not binary.
> 1) default not specified: look up parent chain for default.
> 2) default specified as on. Override parent default.
> 3) default specified as off. Override parent default.

As I understand, your intention is to define inheritance semantics depending
on the default value, and allow children to override this freely with `-` and
`+` semantics. Is that correct?

When I originally proposed the swap cgroup priority mechanism, Michal Koutný
commented that it is unnatural for cgroups if a parent attribute is not
inherited by its child:
(https://lore.kernel.org/linux-mm/rivwhhhkuqy7p4r6mmuhpheaj3c7vcw4w4kavp42avpz7es5vp@hbnvrmgzb5tr/)

Therefore, my current thinking is:
* The global swap setting itself is tier 1 (if nothing is configured).
* If a cgroup has no setting:
  - Top-level cgroups follow the global swap.
  - Child cgroups follow their parent’s setting.
* If a cgroup has its own setting, that setting is applied.
(child cgroups can only select tiers that the parent has allowed.)

This seems natural because most cgroup resource distribution mechanisms follow
a subset inheritance model.

Thus, in my concept, there is no notion of a “default” value that controls
inheritance.

> How are you going to store the list of ranges? Just a bitmask integer
> or a list?

They can be represented as increasing integers, up to 32, and stored as a
bitmask.

> I feel the tier name is more readable. The number to which actual
> device mapping is non trivial to track for humans.

Using increasing integers makes it simpler for the kernel to accept a uniform
interface format, it is identical to the existing cpuset interface, and it
expresses the meaning of “tiers of swap by speed hierarchy” more clearly in my
view.

However, my feeling is still that this approach is clearer both in terms of
implementation and conceptual expression. I would appreciate if you could
reconsider it once more. If after reconsideration you still prefer your
direction, I will follow your decision.

> I want to add another usage case into consideration. The swap.tiers
> does not have to be per cgroup. It can be per VMA. [...]

I understand this as a potential extension use case for swap.tier.  
I will keep this in mind when implementing. If I have further ideas here, I
will share them for discussion.

> Sounds fine. Maybe we can have "ssd:100 zswap:40 hdd" [...]

Yes, this alignment looks good to me!

> Can you elaborate on that. Just brainstorming, can we keep the
> swap.tiers and assign NUMA autobind range to tier as well? [...]

That is actually the same idea I had in mind for the NUMA use case.  
However, I doubt if there is any real workload using this in practice, so I
thought it may be better to leave it out for now. If NUMA autobind is truly
needed later, it could be implemented then.

This point can also be revisited during review or patch writing, so I will
keep thinking about it.

> I feel that that has the risk of  premature optimization. I suggest
> just going with the simplest bitmask check first then optimize as
> follow up when needed. [...]

Yes, I agree with you. Starting with the bitmask implementation seems to be
the right approach.

By the way, while thinking about possible implementation, I would like to ask
your opinion on the following situation:

Suppose a tier has already been defined and cgroups are configured to use it.
Should we allow the tier definition itself to be modified afterwards?

There seem to be two possible choices:

1. Once a cgroup references a tier, modifying that tier should be disallowed.
2. Allow tier re-definition even if cgroups are already referencing it.

Personally, I prefer option (1), since it avoids unexpected changes for
cgroups that already rely on a particular tier definition.

What is your opinion on this?

Best Regards,
Youngjun Park