[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3eb37a6d-ff52-41b0-8f74-d9d049e6ce05@intel.com>
Date: Fri, 3 May 2024 11:23:06 +1200
From: "Huang, Kai" <kai.huang@...el.com>
To: Haitao Huang <haitao.huang@...ux.intel.com>, <jarkko@...nel.org>,
<dave.hansen@...ux.intel.com>, <tj@...nel.org>, <mkoutny@...e.com>,
<linux-kernel@...r.kernel.org>, <linux-sgx@...r.kernel.org>,
<x86@...nel.org>, <cgroups@...r.kernel.org>, <tglx@...utronix.de>,
<mingo@...hat.com>, <bp@...en8.de>, <hpa@...or.com>, <sohil.mehta@...el.com>,
<tim.c.chen@...ux.intel.com>
CC: <zhiquan1.li@...el.com>, <kristen@...ux.intel.com>, <seanjc@...gle.com>,
<zhanb@...rosoft.com>, <anakrish@...rosoft.com>,
<mikko.ylinen@...ux.intel.com>, <yangjie@...rosoft.com>,
<chrisyan@...rosoft.com>
Subject: Re: [PATCH v13 11/14] x86/sgx: Abstract check for global reclaimable
pages
On 1/05/2024 7:51 am, Haitao Huang wrote:
> From: Kristen Carlson Accardi <kristen@...ux.intel.com>
>
> For the global reclaimer to determine if any page available for
> reclamation at the global level, it currently only checks for emptiness
> of the global LRU. That will be inadequate when pages are tracked in
> multiple LRUs, one per cgroup. For this purpose, create a new helper,
> sgx_can_reclaim_global(), to abstract this check. Currently it only
> checks the global LRU, later will check emptiness of LRUs of all cgroups
> when per-cgroup tracking is turned on.
>
> Replace all the checks for emptiness of the global LRU,
> list_empty(&sgx_global_lru.reclaimable), with calls to
> sgx_can_reclaim_global().
>
> Rename sgx_should_reclaim() to sgx_should_reclaim_global() as it is used
> to check if a global reclamation should be performed.
>
> Co-developed-by: Sean Christopherson <sean.j.christopherson@...el.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@...el.com>
> Signed-off-by: Kristen Carlson Accardi <kristen@...ux.intel.com>
> Co-developed-by: Haitao Huang <haitao.huang@...ux.intel.com>
> Signed-off-by: Haitao Huang <haitao.huang@...ux.intel.com>
> Tested-by: Jarkko Sakkinen <jarkko@...nel.org>
> ---
Free free to add:
Reviewed-by: Kai Huang <kai.huang@...el.com>
One thing below:
[...]
> -static bool sgx_should_reclaim(unsigned long watermark)
> +static bool sgx_should_reclaim_global(unsigned long watermark)
> {
> return atomic_long_read(&sgx_nr_free_pages) < watermark &&
> - !list_empty(&sgx_global_lru.reclaimable);
> + sgx_can_reclaim_global();
> }
>
> static void sgx_reclaim_pages_global(struct mm_struct *charge_mm)
> @@ -405,7 +413,7 @@ static void sgx_reclaim_pages_global(struct mm_struct *charge_mm)
> */
> void sgx_reclaim_direct(void)
> {
> - if (sgx_should_reclaim(SGX_NR_LOW_PAGES))
> + if (sgx_should_reclaim_global(SGX_NR_LOW_PAGES))
> sgx_reclaim_pages_global(current->mm);
> }
>
Hmm.. Sorry for not pointing out in the previous version.
Perhaps it makes more sense to do the rename in the patch ...
x86/sgx: Add basic EPC reclamation flow for cgroup
.. where we have actually introduced the concept of per-cgroup reclaim,
and we literally have below change in that patch:
void sgx_reclaim_direct(void)
{
if (sgx_should_reclaim(SGX_NR_LOW_PAGES))
- sgx_reclaim_pages();
+ sgx_reclaim_pages_global();
}
So in that patch, the sgx_should_reclaim() literally just means we
should do gloabl reclaim, but not the per-cgruop reclaim. Thus, perhaps
we just do the renaming here together with the new
sgx_reclaim_pages_global().
If there's a new version needed, please move the renaming to that patch?
Powered by blists - more mailing lists