On Tue, 23 Apr 2024 17:13:15 -0500, Huang, Kai <kai.huang@xxxxxxxxx> wrote:
On Tue, 2024-04-23 at 10:30 -0500, Haitao Huang wrote:
> > It's a workaround because you use the capacity==0 but it does not
really
> > mean to disable the misc cgroup for specific resource IIUC.
>
> Please read the comment around @misc_res_capacity again:
>
> * Miscellaneous resources capacity for the entire machine. 0
capacity
> * means resource is not initialized or not present in the host.
>
I mentioned this in earlier email. I think this means no SGX EPC. It
doesnot mean sgx epc cgroup not enabled. That's also consistent with the
behavior try_charge() fails if capacity is zero.
OK. To me the "capacity" is purely the concept of cgroup, so it must be
interpreted within the scope of "cgroup". If cgroup, in our case, SGX
cgroup, is disabled, then whether "leaving the capacity to reflect the
presence of hardware resource" doesn't really matter.
So what you are saying is that, the kernel must initialize the capacity
of
some MISC resource once it is added as valid type.
And you must initialize the "capacity" even MISC cgroup is disabled
entirely by kernel commandline, in which case, IIUC, misc.capacity is not
even going to show in the /fs.
If this is your point, then your patch:
cgroup/misc: Add SGX EPC resource type
is already broken, because you added the new type w/o initializing the
capacity.
Please fix that up.
> >
> > There is explicit way for user to disable misc without setting
capacity> > to
> > zero.
>
> Which way are you talking about?
Echo "-misc" to cgroup.subtree_control at root level for example still
shows non-zero sgx_epc capacity.
I guess "having to disable all MISC resources just in order to disable
SGX
EPC cgroup" is a brilliant idea.
You can easily disable the entire MISC cgroup by commandline for that
purpose if that's acceptable.
And I have no idea why "still showing non-zero EPC capacity" is important
if SGX cgroup cannot be supported at all.
Okay, all I'm trying to say is we should care about consistency in code
and don't want SGX do something different. Mixing "disable" with
"capacity==0" causes inconsistencies AFAICS:
1) The try_charge() API currently returns error when capacity is zero. So
it appears not to mean that the cgroup is disabled otherwise it should
return success.
2) The current explicit way ("-misc") to disable misc still shows non-zero
entries in misc.capacity. (At least for v2 cgroup, it does when I tested).
Maybe this is not important but I just don't feel good about this
inconsistency.
For now I'll just do BUG_ON() unless there are more strong opinions one
way or the other.
BR
Haitao
