Received-SPF: pass (google.com: domain of linux-kernel+bounces-163230-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Content-Type: text/plain; charset=iso-8859-15; format=flowed; delsp=yes
To: "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>, "mkoutny@suse.com" <mkoutny@suse.com>,
 "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>
Cc: "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: [PATCH v12 12/14] x86/sgx: Turn on per-cgroup EPC reclamation
References: <20240416032011.58578-1-haitao.huang@linux.intel.com>
 <20240416032011.58578-13-haitao.huang@linux.intel.com>
 <524cf9b081d86ae61342fdfc370a3639d0010f94.camel@intel.com>
 <op.2mzyy7ktwjvjmi@hhuan26-mobl.amr.corp.intel.com>
 <5836260d-600d-4802-9f6a-b0bdb19ddbac@intel.com>
Date: Mon, 29 Apr 2024 20:31:21 -0500
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: "Haitao Huang" <haitao.huang@linux.intel.com>
Organization: Intel
Message-ID: <op.2m0o6j2bwjvjmi@hhuan26-mobl.amr.corp.intel.com>
In-Reply-To: <5836260d-600d-4802-9f6a-b0bdb19ddbac@intel.com>
User-Agent: Opera Mail/1.0 (Win32)

On Mon, 29 Apr 2024 17:18:05 -0500, Huang, Kai <kai.huang@intel.com> wrote:

>
>>>>  /*
>>>> @@ -42,7 +63,8 @@ static inline struct sgx_epc_lru_list  
>>>> *sgx_lru_list(struct sgx_epc_page *epc_pag
>>>>   */
>>>>  static inline bool sgx_can_reclaim(void)
>>>>  {
>>>> -    return !list_empty(&sgx_global_lru.reclaimable);
>>>> +    return !sgx_cgroup_lru_empty(misc_cg_root()) ||
>>>> +           !list_empty(&sgx_global_lru.reclaimable);
>>>>  }
>>>
>>> Shouldn't this be:
>>>
>>>     if (IS_ENABLED(CONFIG_CGROUP_MISC))
>>>         return !sgx_cgroup_lru_empty(misc_cg_root());
>>>     else
>>>         return !list_empty(&sgx_global_lru.reclaimable);
>>> ?
>>>
>>> In this way, it is consistent with the sgx_reclaim_pages_global()  
>>> below.
>>>
>>  I changed to this way because sgx_cgroup_lru_empty() is now defined in  
>> both KConfig cases.
>> And it seems better to minimize use of the KConfig variables based on  
>> earlier feedback (some are yours).
>> Don't really have strong preference here. So let me know one way of the  
>> other.
>>
>
> But IMHO your code could be confusing, e.g., it can be interpreted as:
>
>    The EPC pages can be managed by both the cgroup LRUs and the
>    sgx_global_lru simultaneously at runtime when CONFIG_CGROUP_MISC
>    is on.
>
> Which is not true.
>
> So we should make the code clearly reflect the true behaviour.
>

Ok, I'll change back.
Thanks
Haitao