Messages in this thread | | | Subject | Re: [PATCH v7 09/15] x86/sgx: Charge mem_cgroup for per-cgroup reclamation | From | Tim Chen <> | Date | Fri, 02 Feb 2024 15:45:05 -0800 |
| |
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
| |