On Wed, Aug 14, 2024 at 01:22:01PM GMT, Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > Anyway, both use cases make sense to me, disabling writeback > system-wide or in an entire subtree, and disabling writeback on the > root and then selectively enabling it. I am slightly inclined to the > first one (what this patch does). > > Considering the hierarchical cgroup knobs work, we usually use the > most restrictive limit among the ancestors. I guess it ultimately > depends on how we define "most restrictive". Disabling writeback is > restrictive in the sense that you don't have access to free some zswap > space to reclaim more memory. OTOH, disabling writeback also means > that your zswapped memory won't go to disk under memory pressure, so > in that sense it would be restrictive to force writeback :) > > Usually, the "default" is the non-restrictive thing, and then you can > set restrictions that apply to all children (e.g. no limits are set by > default). Since writeback is enabled by default, it seems like the > restriction would be disabling writeback. Hence, it would make sense > to inherit zswap disabling (i.e. only writeback if all ancestors allow > it, like this patch does). > > What we do today dismisses inheritance completely, so it seems to me > like it should be changed anyway. I subscribe to inheritance (at cgroup creation) not proving well (in general). Here's the case of expecting hierarchical semantic of the attribute. With this change -- is there any point in keeping the inheritance around? (Simply default to enabled.) Thanks, Michal
Attachment:
signature.asc
Description: PGP signature
