| Message ID | 20210302081705.1990283-3-vipinsh@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cgroup: New misc cgroup controller | expand |
Hi Vipin, On Tue, 2 Mar 2021 00:17:05 -0800, Vipin Sharma <vipinsh@google.com> wrote: > +Migration and Ownership > +~~~~~~~~~~~~~~~~~~~~~~~ > + > +A miscellaneous scalar resource is charged to the cgroup in which it is > used +first, and stays charged to that cgroup until that resource is > freed. Migrating +a process to a different cgroup does not move the > charge to the destination +cgroup where the process has moved. > + I am trying to see if IOASIDs cgroup can also fit in this misc controller as yet another resource type. https://lore.kernel.org/linux-iommu/20210303131726.7a8cb169@jacob-builder/T/#u However, unlike sev IOASIDs need to be migrated if the process is moved to another cgroup. i.e. charge the destination and uncharge the source. Do you think this behavior can be achieved by differentiating resource types? i.e. add attach callbacks for certain types. Having a single misc interface seems cleaner than creating another controller. Thanks, Jacob
On Wed, Mar 03, 2021 at 06:55:13PM -0800, Jacob Pan wrote: > Hi Vipin, > > On Tue, 2 Mar 2021 00:17:05 -0800, Vipin Sharma <vipinsh@google.com> wrote: > > > +Migration and Ownership > > +~~~~~~~~~~~~~~~~~~~~~~~ > > + > > +A miscellaneous scalar resource is charged to the cgroup in which it is > > used +first, and stays charged to that cgroup until that resource is > > freed. Migrating +a process to a different cgroup does not move the > > charge to the destination +cgroup where the process has moved. > > + > I am trying to see if IOASIDs cgroup can also fit in this misc controller > as yet another resource type. > https://lore.kernel.org/linux-iommu/20210303131726.7a8cb169@jacob-builder/T/#u > However, unlike sev IOASIDs need to be migrated if the process is moved to > another cgroup. i.e. charge the destination and uncharge the source. > > Do you think this behavior can be achieved by differentiating resource > types? i.e. add attach callbacks for certain types. Having a single misc > interface seems cleaner than creating another controller. I think it makes sense to add support for migration for the resources which need it. Resources like SEV, SEV-ES will not participate in migration and won't stop can_attach() to succeed, other resources which need migration will allow or stop based on their limits and capacity in the destination.
Hello, On Wed, Mar 03, 2021 at 10:22:03PM -0800, Vipin Sharma wrote: > > I am trying to see if IOASIDs cgroup can also fit in this misc controller > > as yet another resource type. > > https://lore.kernel.org/linux-iommu/20210303131726.7a8cb169@jacob-builder/T/#u > > However, unlike sev IOASIDs need to be migrated if the process is moved to > > another cgroup. i.e. charge the destination and uncharge the source. > > > > Do you think this behavior can be achieved by differentiating resource > > types? i.e. add attach callbacks for certain types. Having a single misc > > interface seems cleaner than creating another controller. > > I think it makes sense to add support for migration for the resources > which need it. Resources like SEV, SEV-ES will not participate in > migration and won't stop can_attach() to succeed, other resources which > need migration will allow or stop based on their limits and capacity in > the destination. Please note that cgroup2 by and large don't really like or support charge migration or even migrations themselves. We tried that w/ memcg on cgroup1 and it turned out horrible. The expected usage model as decribed in the doc is using migration to seed a cgroup (or even better, use the new clone call to start in the target cgroup) and then stay there until exit. All existing controllers assume this usage model and I'm likely to nack deviation unless there are some super strong justifications. Thanks.
Hi Vipin & Tejun, Sorry for the late reply, I sent from a different email address than I intended. Please see my comments inline. On Thu, 4 Mar 2021 03:51:16 -0500, Tejun Heo <tj@kernel.org> wrote: > Hello, > > On Wed, Mar 03, 2021 at 10:22:03PM -0800, Vipin Sharma wrote: > > > I am trying to see if IOASIDs cgroup can also fit in this misc > > > controller as yet another resource type. > > > https://lore.kernel.org/linux-iommu/20210303131726.7a8cb169@jacob-builder/T/#u > > > However, unlike sev IOASIDs need to be migrated if the process is > > > moved to another cgroup. i.e. charge the destination and uncharge the > > > source. > > > > > > Do you think this behavior can be achieved by differentiating resource > > > types? i.e. add attach callbacks for certain types. Having a single > > > misc interface seems cleaner than creating another controller. > > > > I think it makes sense to add support for migration for the resources > > which need it. Resources like SEV, SEV-ES will not participate in > > migration and won't stop can_attach() to succeed, other resources which > > need migration will allow or stop based on their limits and capacity in > > the destination. > Sounds good. Perhaps some capability/feature flags for each resource such that different behavior can be accommodated? Could you please include me in your future posting? I will rebase on yours. > Please note that cgroup2 by and large don't really like or support charge > migration or even migrations themselves. We tried that w/ memcg on cgroup1 > and it turned out horrible. The expected usage model as decribed in the > doc is using migration to seed a cgroup (or even better, use the new > clone call to start in the target cgroup) and then stay there until exit. > All existing controllers assume this usage model and I'm likely to nack > deviation unless there are some super strong justifications. > Thank you so much for the pointers. Just to be clear, you meant 1. Use clone3 CLONE_INTO_CGROUP to put the child into a different cgroup. 2. Do not support migration of the parent (existing proc) Thanks, Jacob
On Fri, Mar 12, 2021 at 12:58:21PM -0800, Jacob Pan wrote: > Hi Vipin & Tejun, > > Sorry for the late reply, I sent from a different email address than I > intended. Please see my comments inline. > > > On Thu, 4 Mar 2021 03:51:16 -0500, Tejun Heo <tj@kernel.org> wrote: > > > Hello, > > > > On Wed, Mar 03, 2021 at 10:22:03PM -0800, Vipin Sharma wrote: > > > > I am trying to see if IOASIDs cgroup can also fit in this misc > > > > controller as yet another resource type. > > > > https://lore.kernel.org/linux-iommu/20210303131726.7a8cb169@jacob-builder/T/#u > > > > However, unlike sev IOASIDs need to be migrated if the process is > > > > moved to another cgroup. i.e. charge the destination and uncharge the > > > > source. > > > > > > > > Do you think this behavior can be achieved by differentiating resource > > > > types? i.e. add attach callbacks for certain types. Having a single > > > > misc interface seems cleaner than creating another controller. > > > > > > I think it makes sense to add support for migration for the resources > > > which need it. Resources like SEV, SEV-ES will not participate in > > > migration and won't stop can_attach() to succeed, other resources which > > > need migration will allow or stop based on their limits and capacity in > > > the destination. > > > Sounds good. Perhaps some capability/feature flags for each resource such > that different behavior can be accommodated? > Could you please include me in your future posting? I will rebase on yours. Hi Jacob Based on Tejun's response, I will not add charge migration support in misc controller. I can definitly add you in my future posting, if you still wanna use it without charge migration support. Thanks Vipin
Hi Vipin, On Fri, 12 Mar 2021 13:15:14 -0800, Vipin Sharma <vipinsh@google.com> wrote: > On Fri, Mar 12, 2021 at 12:58:21PM -0800, Jacob Pan wrote: > > Hi Vipin & Tejun, > > > > Sorry for the late reply, I sent from a different email address than I > > intended. Please see my comments inline. > > > > > > On Thu, 4 Mar 2021 03:51:16 -0500, Tejun Heo <tj@kernel.org> wrote: > > > > > Hello, > > > > > > On Wed, Mar 03, 2021 at 10:22:03PM -0800, Vipin Sharma wrote: > > > > > I am trying to see if IOASIDs cgroup can also fit in this misc > > > > > controller as yet another resource type. > > > > > https://lore.kernel.org/linux-iommu/20210303131726.7a8cb169@jacob-builder/T/#u > > > > > However, unlike sev IOASIDs need to be migrated if the process is > > > > > moved to another cgroup. i.e. charge the destination and uncharge > > > > > the source. > > > > > > > > > > Do you think this behavior can be achieved by differentiating > > > > > resource types? i.e. add attach callbacks for certain types. > > > > > Having a single misc interface seems cleaner than creating > > > > > another controller. > > > > > > > > I think it makes sense to add support for migration for the > > > > resources which need it. Resources like SEV, SEV-ES will not > > > > participate in migration and won't stop can_attach() to succeed, > > > > other resources which need migration will allow or stop based on > > > > their limits and capacity in the destination. > > > > > Sounds good. Perhaps some capability/feature flags for each resource > > such that different behavior can be accommodated? > > Could you please include me in your future posting? I will rebase on > > yours. > > Hi Jacob > > Based on Tejun's response, I will not add charge migration support in > misc controller. > Sounds good. I need some confirmation on whether migration is a must have for VMs allocated IOASIDs. Our primary goal is to limit the amount of IOASIDs that VMs can allocate. If a VM is migrated to a different cgroup, I think we need to charge/uncharge the destination/source cgroup in order enforce the limit. I am not an expert here, any feedback would be appreciated. > I can definitly add you in my future posting, if you still wanna use it > without charge migration support. > Yes, please. I got your v3 already, so just future patches. > Thanks > Vipin Thanks, Jacob
On Fri, Mar 12, 2021 at 02:59:04PM -0800, Jacob Pan wrote: > Our primary goal is to limit the amount of IOASIDs that VMs can allocate. > If a VM is migrated to a different cgroup, I think we need to > charge/uncharge the destination/source cgroup in order enforce the limit. I > am not an expert here, any feedback would be appreciated. That simply isn't a supported usage model. None of other resources will get tracked if you do that. Thanks.
Hi Tejun, On Sat, 13 Mar 2021 05:20:39 -0500, Tejun Heo <tj@kernel.org> wrote: > On Fri, Mar 12, 2021 at 02:59:04PM -0800, Jacob Pan wrote: > > Our primary goal is to limit the amount of IOASIDs that VMs can > > allocate. If a VM is migrated to a different cgroup, I think we need to > > charge/uncharge the destination/source cgroup in order enforce the > > limit. I am not an expert here, any feedback would be appreciated. > > That simply isn't a supported usage model. None of other resources will > get tracked if you do that. Isn't PIDs controller doing the charge/uncharge? I was under the impression that each resource can be independently charged/uncharged, why it affects other resources? Sorry for the basic question. I also didn't quite get the limitation on cgroup v2 migration, this is much simpler than memcg. Could you give me some pointers? BTW, since the IOASIDs are used to tag DMA and bound with guest process(mm) for shared virtual addressing. fork() cannot be supported, so I guess clone is not a solution here. Thanks, Jacob
Hello, On Sat, Mar 13, 2021 at 08:57:01AM -0800, Jacob Pan wrote: > Isn't PIDs controller doing the charge/uncharge? I was under the impression > that each resource can be independently charged/uncharged, why it affects > other resources? Sorry for the basic question. Yeah, PID is an exception as we needed the initial migration to seed new cgroups and it gets really confusing with other ways to observe the processes - e.g. if you follow the original way of creating a cgroup, forking and then moving the seed process into the target cgroup, if we don't migrate the pid charge together, the numbers wouldn't agree and the seeder cgroup may end up running out of pids if there are any restrictions. > I also didn't quite get the limitation on cgroup v2 migration, this is much > simpler than memcg. Could you give me some pointers? Migration itself doesn't have restrictions but all resources are distributed on the same hierarchy, so the controllers are supposed to follow the same conventions that can be implemented by all controllers. > BTW, since the IOASIDs are used to tag DMA and bound with guest process(mm) > for shared virtual addressing. fork() cannot be supported, so I guess clone > is not a solution here. Can you please elaborate what wouldn't work? The new spawning into a new cgroup w/ clone doesn't really change the usage model. It's just a neater way to seed a new cgroup. If you're saying that the overall usage model doesn't fit the needs of IOASIDs, it likely shouldn't be a cgroup controller. Thanks.
Hi Tejun, On Sat, 13 Mar 2021 13:05:36 -0500, Tejun Heo <tj@kernel.org> wrote: > Hello, > > On Sat, Mar 13, 2021 at 08:57:01AM -0800, Jacob Pan wrote: > > Isn't PIDs controller doing the charge/uncharge? I was under the > > impression that each resource can be independently charged/uncharged, > > why it affects other resources? Sorry for the basic question. > > Yeah, PID is an exception as we needed the initial migration to seed new > cgroups and it gets really confusing with other ways to observe the > processes - e.g. if you follow the original way of creating a cgroup, > forking and then moving the seed process into the target cgroup, if we > don't migrate the pid charge together, the numbers wouldn't agree and the > seeder cgroup may end up running out of pids if there are any > restrictions. > Thanks for explaining. Unfortunately, it seems IOASIDs has a similar needs in terms of migrating the charge. > > I also didn't quite get the limitation on cgroup v2 migration, this is > > much simpler than memcg. Could you give me some pointers? > > Migration itself doesn't have restrictions but all resources are > distributed on the same hierarchy, so the controllers are supposed to > follow the same conventions that can be implemented by all controllers. > Got it, I guess that is the behavior required by the unified hierarchy. Cgroup v1 would be ok? But I am guessing we are not extending on v1? > > BTW, since the IOASIDs are used to tag DMA and bound with guest > > process(mm) for shared virtual addressing. fork() cannot be supported, > > so I guess clone is not a solution here. > > Can you please elaborate what wouldn't work? The new spawning into a new > cgroup w/ clone doesn't really change the usage model. It's just a neater > way to seed a new cgroup. If you're saying that the overall usage model > doesn't fit the needs of IOASIDs, it likely shouldn't be a cgroup > controller. > The IOASIDs are programmed into devices to generate DMA requests tagged with them. The IOMMU has a per device IOASID table with each entry has two pointers: - the PGD of the guest process. - the PGD of the host process The result of this 2 stage/nested translation is that we can share virtual address (SVA) between guest process and DMA. The host process needs to allocate multiple IOASIDs since one IOASID is needed for each guest process who wants SVA. The DMA binding among device-IOMMU-process is setup via a series of user APIs (e.g. via VFIO). If a process calls fork(), the children does not inherit the IOASIDs and their bindings. Children who wish to use SVA has to call those APIs to establish the binding for themselves. Therefore, if a host process allocates 10 IOASIDs then does a fork()/clone(), it cannot charge 10 IOASIDs in the new cgroup. i.e. the 10 IOASIDs stays with the process wherever it goes. I feel this fit in the domain model, true? > Thanks. > Thanks, Jacob
Hello, On Mon, Mar 15, 2021 at 03:11:55PM -0700, Jacob Pan wrote: > > Migration itself doesn't have restrictions but all resources are > > distributed on the same hierarchy, so the controllers are supposed to > > follow the same conventions that can be implemented by all controllers. > > > Got it, I guess that is the behavior required by the unified hierarchy. > Cgroup v1 would be ok? But I am guessing we are not extending on v1? A new cgroup1 only controller is unlikely to be accpeted. > The IOASIDs are programmed into devices to generate DMA requests tagged > with them. The IOMMU has a per device IOASID table with each entry has two > pointers: > - the PGD of the guest process. > - the PGD of the host process > > The result of this 2 stage/nested translation is that we can share virtual > address (SVA) between guest process and DMA. The host process needs to > allocate multiple IOASIDs since one IOASID is needed for each guest process > who wants SVA. > > The DMA binding among device-IOMMU-process is setup via a series of user > APIs (e.g. via VFIO). > > If a process calls fork(), the children does not inherit the IOASIDs and > their bindings. Children who wish to use SVA has to call those APIs to > establish the binding for themselves. > > Therefore, if a host process allocates 10 IOASIDs then does a > fork()/clone(), it cannot charge 10 IOASIDs in the new cgroup. i.e. the 10 > IOASIDs stays with the process wherever it goes. > > I feel this fit in the domain model, true? I still don't get where migration is coming into the picture. Who's migrating where? Thanks.
Hi Tejun, On Mon, 15 Mar 2021 18:19:35 -0400, Tejun Heo <tj@kernel.org> wrote: > Hello, > > On Mon, Mar 15, 2021 at 03:11:55PM -0700, Jacob Pan wrote: > > > Migration itself doesn't have restrictions but all resources are > > > distributed on the same hierarchy, so the controllers are supposed to > > > follow the same conventions that can be implemented by all > > > controllers. > > Got it, I guess that is the behavior required by the unified hierarchy. > > Cgroup v1 would be ok? But I am guessing we are not extending on v1? > > A new cgroup1 only controller is unlikely to be accpeted. > > > The IOASIDs are programmed into devices to generate DMA requests tagged > > with them. The IOMMU has a per device IOASID table with each entry has > > two pointers: > > - the PGD of the guest process. > > - the PGD of the host process > > > > The result of this 2 stage/nested translation is that we can share > > virtual address (SVA) between guest process and DMA. The host process > > needs to allocate multiple IOASIDs since one IOASID is needed for each > > guest process who wants SVA. > > > > The DMA binding among device-IOMMU-process is setup via a series of user > > APIs (e.g. via VFIO). > > > > If a process calls fork(), the children does not inherit the IOASIDs and > > their bindings. Children who wish to use SVA has to call those APIs to > > establish the binding for themselves. > > > > Therefore, if a host process allocates 10 IOASIDs then does a > > fork()/clone(), it cannot charge 10 IOASIDs in the new cgroup. i.e. the > > 10 IOASIDs stays with the process wherever it goes. > > > > I feel this fit in the domain model, true? > > I still don't get where migration is coming into the picture. Who's > migrating where? > Sorry, perhaps I can explain by an example. There are two cgroups: cg_A and cg_B with limit set to 20 for both. Process1 is in cg_A. The initial state is: cg_A/ioasid.current=0, cg_A/ioasid.max=20 cg_B/ioasid.current=0, cg_B/ioasid.max=20 Now, consider the following steps: 1. Process1 allocated 10 IOASIDs, cg_A/ioasid.current=10, cg_B/ioasid.current=0 2. then we want to move/migrate Process1 to cg_B. so we need uncharge 10 of cg_A, charge 10 of cg_B 3. After the migration, I expect cg_A/ioasid.current=0, cg_B/ioasid.current=10 We don't enforce the limit during this organizational change since we can't force free IOASIDs. But any new allocations will be subject to the limit set in ioasid.max. > Thanks. > Thanks, Jacob
Hello, On Mon, Mar 15, 2021 at 04:40:12PM -0700, Jacob Pan wrote: > 2. then we want to move/migrate Process1 to cg_B. so we need uncharge 10 of > cg_A, charge 10 of cg_B So, what I don't get is why this migration is necessary. This isn't supported as a usage pattern and no one, at least in terms of wide-spread usage, does this. Why is this a requirement for your use case? Thanks.
Hi Tejun, On Mon, 15 Mar 2021 19:54:36 -0400, Tejun Heo <tj@kernel.org> wrote: > Hello, > > On Mon, Mar 15, 2021 at 04:40:12PM -0700, Jacob Pan wrote: > > 2. then we want to move/migrate Process1 to cg_B. so we need uncharge > > 10 of cg_A, charge 10 of cg_B > > So, what I don't get is why this migration is necessary. This isn't > supported as a usage pattern and no one, at least in terms of wide-spread > usage, does this. Why is this a requirement for your use case? > I don't know if this is required. I thought utilities such as cgclassify need to be supported. " cgclassify - move running task(s) to given cgroups " If no such use case, I am fine with dropping the migration support. Just enforce limit on allocations. > Thanks. > Thanks, Jacob
On Mon, Mar 15, 2021 at 06:30:30PM -0700, Jacob Pan wrote: > I don't know if this is required. I thought utilities such as cgclassify > need to be supported. > " cgclassify - move running task(s) to given cgroups " > If no such use case, I am fine with dropping the migration support. Just > enforce limit on allocations. Yeah, that's what all other controllers do. Please read the in-tree cgroup2 doc. Thanks.
Hi Tejun, On Mon, 15 Mar 2021 22:22:12 -0400, Tejun Heo <tj@kernel.org> wrote: > On Mon, Mar 15, 2021 at 06:30:30PM -0700, Jacob Pan wrote: > > I don't know if this is required. I thought utilities such as cgclassify > > need to be supported. > > " cgclassify - move running task(s) to given cgroups " > > If no such use case, I am fine with dropping the migration support. Just > > enforce limit on allocations. > > Yeah, that's what all other controllers do. Please read the in-tree > cgroup2 doc. > Thanks for your patience and guidance, will try to merge with misc controller and go from there. Thanks, Jacob
diff --git a/Documentation/admin-guide/cgroup-v1/index.rst b/Documentation/admin-guide/cgroup-v1/index.rst index 226f64473e8e..99fbc8a64ba9 100644 --- a/Documentation/admin-guide/cgroup-v1/index.rst +++ b/Documentation/admin-guide/cgroup-v1/index.rst @@ -17,6 +17,7 @@ Control Groups version 1 hugetlb memcg_test memory + misc net_cls net_prio pids diff --git a/Documentation/admin-guide/cgroup-v1/misc.rst b/Documentation/admin-guide/cgroup-v1/misc.rst new file mode 100644 index 000000000000..661614c24df3 --- /dev/null +++ b/Documentation/admin-guide/cgroup-v1/misc.rst @@ -0,0 +1,4 @@ +=============== +Misc controller +=============== +Please refer "Misc" documentation in Documentation/admin-guide/cgroup-v2.rst diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index 1de8695c264b..74777323b7fd 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -63,8 +63,11 @@ v1 is available under :ref:`Documentation/admin-guide/cgroup-v1/index.rst <cgrou 5-7-1. RDMA Interface Files 5-8. HugeTLB 5.8-1. HugeTLB Interface Files - 5-8. Misc - 5-8-1. perf_event + 5-9. Misc + 5.9-1 Miscellaneous cgroup Interface Files + 5.9-2 Migration and Ownership + 5-10. Others + 5-10-1. perf_event 5-N. Non-normative information 5-N-1. CPU controller root cgroup process behaviour 5-N-2. IO controller root cgroup process behaviour @@ -2163,6 +2166,68 @@ HugeTLB Interface Files Misc ---- +The Miscellaneous cgroup provides the resource limiting and tracking +mechanism for the scalar resources which cannot be abstracted like the other +cgroup resources. Controller is enabled by the CONFIG_CGROUP_MISC config +option. + +The first two resources added to the miscellaneous controller are Secure +Encrypted Virtualization (SEV) ASIDs and SEV - Encrypted State (SEV-ES) ASIDs. +These limited ASIDs are used for encrypting virtual machines memory on the AMD +platform. + +Misc Interface Files +~~~~~~~~~~~~~~~~~~~~ + +Miscellaneous controller provides 3 interface files: + + misc.capacity + A read-only flat-keyed file shown only in the root cgroup. It shows + miscellaneous scalar resources available on the platform along with + their quantities:: + + $ cat misc.capacity + sev 50 + sev_es 10 + + misc.current + A read-only flat-keyed file shown in the non-root cgroups. It shows + the current usage of the resources in the cgroup and its children.:: + + $ cat misc.current + sev 3 + sev_es 0 + + misc.max + A read-write flat-keyed file shown in the non root cgroups. Allowed + maximum usage of the resources in the cgroup and its children.:: + + $ cat misc.max + sev max + sev_es 4 + + Limit can be set by:: + + # echo sev 1 > misc.max + + Limit can be set to max by:: + + # echo sev max > misc.max + + Limits can be set higher than the capacity value in the misc.capacity + file. + +Migration and Ownership +~~~~~~~~~~~~~~~~~~~~~~~ + +A miscellaneous scalar resource is charged to the cgroup in which it is used +first, and stays charged to that cgroup until that resource is freed. Migrating +a process to a different cgroup does not move the charge to the destination +cgroup where the process has moved. + +Others +------ + perf_event ~~~~~~~~~~