Received-SPF: pass (google.com: domain of linux-kernel+bounces-139203-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Content-Type: text/plain; charset=iso-8859-15; format=flowed; delsp=yes
To: =?iso-8859-15?Q?Michal_Koutn=FD?= <mkoutny@suse.com>, "Haitao Huang"
 <haitao.huang@linux.intel.com>
Cc: "hpa@zytor.com" <hpa@zytor.com>, "tim.c.chen@linux.intel.com"
 <tim.c.chen@linux.intel.com>, "linux-sgx@vger.kernel.org"
 <linux-sgx@vger.kernel.org>, "x86@kernel.org" <x86@kernel.org>,
 "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
 "jarkko@kernel.org" <jarkko@kernel.org>, "cgroups@vger.kernel.org"
 <cgroups@vger.kernel.org>, "linux-kernel@vger.kernel.org"
 <linux-kernel@vger.kernel.org>, "tglx@linutronix.de" <tglx@linutronix.de>,
 "Mehta, Sohil" <sohil.mehta@intel.com>, "tj@kernel.org" <tj@kernel.org>,
 "mingo@redhat.com" <mingo@redhat.com>, "bp@alien8.de" <bp@alien8.de>, "Huang,
 Kai" <kai.huang@intel.com>, "mikko.ylinen@linux.intel.com"
 <mikko.ylinen@linux.intel.com>, "seanjc@google.com" <seanjc@google.com>,
 "anakrish@microsoft.com" <anakrish@microsoft.com>, "Zhang, Bo"
 <zhanb@microsoft.com>, "kristen@linux.intel.com" <kristen@linux.intel.com>,
 "yangjie@microsoft.com" <yangjie@microsoft.com>, "Li, Zhiquan1"
 <zhiquan1.li@intel.com>, "chrisyan@microsoft.com" <chrisyan@microsoft.com>
Subject: Re: Re: [PATCH v10 12/14] x86/sgx: Turn on per-cgroup EPC reclamation
References: <20240328002229.30264-1-haitao.huang@linux.intel.com>
 <20240328002229.30264-13-haitao.huang@linux.intel.com>
 <d25dbe76d48a0b6c74fa09b06f1ca3fdf234a190.camel@intel.com>
 <op.2lw8gfg2wjvjmi@hhuan26-mobl.amr.corp.intel.com>
 <7a21c0de-ba59-4e76-8d67-70957f9db581@intel.com>
 <op.2lx047lrwjvjmi@hhuan26-mobl.amr.corp.intel.com>
 <imqjuxgmm5updfnl75molzixlq52ttlufd6sn2vpcevaby5l7f@22j23whlbvux>
 <op.2lyv641jwjvjmi@hhuan26-mobl.amr.corp.intel.com>
Date: Wed, 10 Apr 2024 13:28:06 -0500
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: Quoted-Printable
From: "Haitao Huang" <haitao.huang@linux.intel.com>
Organization: Intel
Message-ID: <op.2l0yw4vnwjvjmi@hhuan26-mobl.amr.corp.intel.com>
In-Reply-To: <op.2lyv641jwjvjmi@hhuan26-mobl.amr.corp.intel.com>
User-Agent: Opera Mail/1.0 (Win32)

On Tue, 09 Apr 2024 10:34:06 -0500, Haitao Huang  =

<haitao.huang@linux.intel.com> wrote:

> On Tue, 09 Apr 2024 04:03:22 -0500, Michal Koutn=FD <mkoutny@suse.com>=
  =

> wrote:
>
>> On Mon, Apr 08, 2024 at 11:23:21PM -0500, Haitao Huang  =

>> <haitao.huang@linux.intel.com> 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 N=
ULL
>>> regardless 'cgroup_disable=3Dmisc'.
>>>
>>> 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 controlle=
r.)
>>
>> 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 st=
ore
>> 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