Re: Re: [PATCH v10 12/14] x86/sgx: Turn on per-cgroup EPC reclamation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 09 Apr 2024 10:34:06 -0500, Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx> wrote:

On Tue, 09 Apr 2024 04:03:22 -0500, Michal Koutný <mkoutny@xxxxxxxx> wrote:

On Mon, Apr 08, 2024 at 11:23:21PM -0500, Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx> wrote:
It's always non-NULL based on testing.

It's hard for me to say definitely by reading the code. But IIUC
cgroup_disable command-line only blocks operations in /sys/fs/cgroup so user space can't set up controllers and config limits, etc., for the diasabled ones. Each task->cgroups would still have a non-NULL pointer to the static
root object for each cgroup that is enabled by KConfig, so
get_current_misc_cg() thus  sgx_get_current_cg() should not return NULL
regardless 'cgroup_disable=misc'.

Maybe @Michal or @tj can confirm?

The current implementation creates root css object (see cgroup_init(),
cgroup_ssid_enabled() check is after cgroup_init_subsys()).
I.e. it will look like all tasks are members of root cgroup wrt given
controller permanently and controller attribute files won't exist.

(It is up to the controller implementation to do further optimization
based on the boot-time disablement (e.g. see uses of
mem_cgroup_disabled()). Not sure if this is useful for misc controller.)

As for the WARN_ON(1), taking example from memcg -- NULL is best
synonymous with root. It's a judgement call which of the values to store
and when to intepret it.

HTH,
Michal
Thanks for the info.

The way I see it, misc does not have special handling like memcg so every task at least belong to the root(default) group even if it's disabled by command line parameter. So we would not get NULL from get_current_misc_cg(). I think I'll keep the WARN_ON_ONCE for now as a reminder in case misc do have custom support for disabling in future.

Actually I think it makes more sense just add some comments instead of WARN.
That's what I did in v11 now.

Thanks
Haitao





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux