Re: [Patch v3 0/2] cgroup: KVM: New Encryption IDs cgroup controller

From: Vipin Sharma
Date: Wed Dec 16 2020 - 15:03:49 EST


On Wed, Dec 9, 2020 at 12:59 PM Tejun Heo <tj@xxxxxxxxxx> wrote:
> * I don't have an overall objection. In terms of behavior, the only thing
> which stood out was input rejection depending on the current usage. The
> preferred way of handling that is rejecting future allocations rather than
> failing configuration as that makes it impossible e.g. to lower limit and
> drain existing usages from outside the container.

Thanks. In next version of the patch I will remove rejection of max value
based on current usage.

On Wed, Dec 16, 2020 at 7:27 AM Tejun Heo <tj@xxxxxxxxxx> wrote:
> On Thu, Dec 10, 2020 at 03:44:35PM -0800, David Rientjes wrote:
> > Concern with a single misc controller would be that any subsystem that
> > wants to use it has to exactly fit this support: current, max, stat,
> > nothing more. The moment a controller needs some additional support, and
> > its controller is already implemented in previous kernel versionv as a
> > part of "misc," we face a problem.
>
> Yeah, that's a valid concern, but given the history, there doesn't seem to
> be much need beyond that for these use cases and the limited need seems
> inherent to the way the resources are defined and consumed. So yeah, it can
> either way.

I think a misc controller should be able support other "Resource Distribution
Models" mentioned in the cgroup v2 documentation besides limits. There might be
use cases in future which want to use weight, protection or allocation models.
If that happens it will be more difficult to support these different resources.
This will also mean the same hierarchy might get charged differently by the
same controller.

I like the idea of having a separate controller to keep the code simple and
easier for maintenance.

If you decide to have a separate misc controller please let me know what will
be the overall expectations and I can change my patch to reflect that,
otherwise I can send out a new patch with just removal of max input rejection.

My understanding with a "misc" controller is it will be something like
- cgroup v2
cgroup/misc.encryption_ids.{sev, tdx, seid}.{stat, max, current}

- cgroup v1
cgroup/misc/misc.encryption_ids.{sev, tdx, seid}.{stat, max, current}

Thanks
Vipin