Re: [RFC PATCH v3] zswap: memcontrol: implement zswap writeback disabling

From: Nhat Pham
Date: Wed Nov 08 2023 - 22:17:43 EST


On Wed, Nov 8, 2023 at 6:41 PM Chris Li <chrisl@xxxxxxxxxx> wrote:
>
> Hi Nhant,
>
> On Fri, Nov 3, 2023 at 12:24 PM Nhat Pham <nphamcs@xxxxxxxxx> wrote:
> > >
> > > Would it be more convenient if the initial value is inherited from the
> > > parent (the root starts with true)?
> > >
> > > I can see this being useful if we want to set it to false on the
> > > entire machine or one a parent cgroup, we can set it before creating
> > > any children instead of setting it to 0 every time we create a new
> > > cgroup.
> >
> > I'm not 100% sure about the benefit or have a strong opinion one way
> > or another, but this sounds like a nice-to-have detail to me, and a relatively
> > low cost one (both in effort and at runtime) at that too.
> >
> > Propagating the change everytime we modify the memory.zswap.writeback
> > value of the ancestor might be data race-prone (and costly, depending on
> > how big the cgroup subtree is), but this is just a one-time-per-cgroup
> > propagation (at the new cgroup creation time).
>
> I think Yosary was suggesting inheriting the initial value from
> parents. That is just one level look up when you create the new
> cgroup, using the parent value as default. No recursive.
> What you described above seems different to me. I understand what you
> are suggesting is that writing to the parent cgroup will recursively
> write to all child cgroup.
> >
> > Can anyone come up with a failure case for this change, or why it might be
> > a bad idea?
>
> I would suggest against recursive changing value behavior.
> What if you want the parent but also want the child to keep its value
> not changed? Every change to the parent will have to go through the
> child to flip it back.
> Inherit from the parent seems fine.
>
> Chris

Hi Chris,

I've actually sent out a new version of the patch series implementing
(what I believed to be) Yosry's suggestion. Feel free to take a look!

https://lore.kernel.org/all/20231106231158.380730-1-nphamcs@xxxxxxxxx/

Here, I was actually agreeing with you - recursive propagation would
not be a good idea.

Best,
Nhat