Re: [PATCH v9 10/15] x86/sgx: Add EPC reclamation in cgroup try_charge()

Huang, Kai
2024-02-21T11:06:07Z

> -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?