On Mon, 2024-01-22 at 09:20 -0800, Haitao Huang wrote:
>
> @@ -1047,29 +1037,38 @@ static struct mem_cgroup
> *sgx_encl_get_mem_cgroup(struct sgx_encl *encl)
> * @encl: an enclave pointer
> * @page_index: enclave page index
> * @backing: data for accessing backing storage for the page
> + * @indirect: in ksgxd or EPC cgroup work queue context
> + *
> + * Create a backing page for loading data back into an EPC page with ELDU.
> This function takes
> + * a reference on a new backing page which must be dropped with a
> corresponding call to
> + * sgx_encl_put_backing().
> *
> - * When called from ksgxd, sets the active memcg from one of the
> - * mms in the enclave's mm_list prior to any backing page allocation,
> - * in order to ensure that shmem page allocations are charged to the
> - * enclave. Create a backing page for loading data back into an EPC page
> with
> - * ELDU. This function takes a reference on a new backing page which
> - * must be dropped with a corresponding call to sgx_encl_put_backing().
> + * When @indirect is true, sets the active memcg from one of the mms in the
> enclave's mm_list
> + * prior to any backing page allocation, in order to ensure that shmem page
> allocations are
> + * charged to the enclave.
> *
> * Return:
> * 0 on success,
> * -errno otherwise.
> */
> int sgx_encl_alloc_backing(struct sgx_encl *encl, unsigned long page_index,
> - struct sgx_backing *backing)
> + struct sgx_backing *backing, bool indirect)
> {
> - struct mem_cgroup *encl_memcg = sgx_encl_get_mem_cgroup(encl);
> - struct mem_cgroup *memcg = set_active_memcg(encl_memcg);
> + struct mem_cgroup *encl_memcg;
> + struct mem_cgroup *memcg;
> int ret;
>
> + if (indirect) {
> + encl_memcg = sgx_encl_get_mem_cgroup(encl);
> + memcg = set_active_memcg(encl_memcg);
> + }
> +
> ret = __sgx_encl_get_backing(encl, page_index, backing);
>
> - set_active_memcg(memcg);
> - mem_cgroup_put(encl_memcg);
> + if (indirect) {
> + set_active_memcg(memcg);
> + mem_cgroup_put(encl_memcg);
> + }
>
You can reduce the number of if statements to make the logic
simpler. Something like
if (!indirect)
return __sgx_encl_get_backing(encl, page_index, backing);
encl_memcg = sgx_encl_get_mem_cgroup(encl);
memcg = set_active_memcg(encl_memcg);
ret = __sgx_encl_get_backing(encl, page_index, backing);
set_active_memcg(memcg);
mem_cgroup_put(encl_memcg);
> return ret;
Tim
