On Thu, 22 Feb 2024 15:26:05 -0600, Huang, Kai <kai.huang@xxxxxxxxx> wrote:
On 23/02/2024 6:09 am, Haitao Huang wrote:
On Wed, 21 Feb 2024 05:06:02 -0600, Huang, Kai <kai.huang@xxxxxxxxx>
wrote:
-int sgx_epc_cgroup_try_charge(struct sgx_epc_cgroup *epc_cg)
+int sgx_epc_cgroup_try_charge(struct sgx_epc_cgroup *epc_cg, bool
reclaim)
{
- return misc_cg_try_charge(MISC_CG_RES_SGX_EPC, epc_cg->cg,
PAGE_SIZE);
+ for (;;) {
+ if (!misc_cg_try_charge(MISC_CG_RES_SGX_EPC, epc_cg->cg,
+ PAGE_SIZE))
+ break;
+
+ if (sgx_epc_cgroup_lru_empty(epc_cg->cg))
+ return -ENOMEM;
+
+ if (signal_pending(current))
+ return -ERESTARTSYS;
+
+ if (!reclaim) {
+ queue_work(sgx_epc_cg_wq, &epc_cg->reclaim_work);
+ return -EBUSY;
+ }
+
+ if (!sgx_epc_cgroup_reclaim_pages(epc_cg->cg, false))
+ /* All pages were too young to reclaim, try again a
little later */
+ schedule();
+ }
+
+ return 0;
}
Seems this code change is 90% similar to the existing code in the
sgx_alloc_epc_page():
...
for ( ; ; ) {
page = __sgx_alloc_epc_page();
if (!IS_ERR(page)) {
page->owner = owner;
break;
}
if (list_empty(&sgx_active_page_list))
return ERR_PTR(-ENOMEM);
if (!reclaim) {
page = ERR_PTR(-EBUSY);
break;
}
if (signal_pending(current)) {
page = ERR_PTR(-ERESTARTSYS);
break;
}
sgx_reclaim_pages();
cond_resched();
}
...
Is it better to move the logic/code change in try_charge() out to
sgx_alloc_epc_page() to unify them?
IIUC, the logic is quite similar: When you either failed to allocate
one page,
or failed to charge one page, you try to reclaim EPC page(s) from the
current
EPC cgroup, either directly or indirectly.
No?
Only these lines are the same:
if (!reclaim) {
page = ERR_PTR(-EBUSY);
break;
}
if (signal_pending(current)) {
page = ERR_PTR(-ERESTARTSYS);
break;
}
In sgx_alloc_epc_page() we do global reclamation but here we do
per-cgroup reclamation.
But why? If we failed to allocate, shouldn't we try to reclaim from the
_current_ EPC cgroup instead of global? E.g., I thought one enclave in
one EPC cgroup requesting insane amount of EPC shouldn't impact enclaves
inside other cgroups?
Right. When code reaches to here, we already passed reclaim per cgroup.
The cgroup may not at or reach limit but system has run out of physical
EPC.
Thanks
Haitao
