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

[Huang, Kai]

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