On Wed Jul 12, 2023 at 11:01 PM UTC, Haitao Huang wrote: > In a later patch, when a cgroup has exceeded the max capacity for EPC pages > and there are no more Enclave EPC pages associated with the cgroup that can > be reclaimed, the only pages still associated with an enclave will be the > unreclaimable Version Array (VA) pages or SECS pages, and the entire > enclave will need to be killed to free up those pages. > > Currently, given an enclave pointer it is easy to find the associated VA > pages and free them, however, OOM killing an enclave based on cgroup limits > will require examining a cgroup's unreclaimable page list, and finding an > enclave given a SECS page or a VA page. This will require a backpointer > from a page to an enclave, including for VA pages. > > When allocating new Version Array (VA) pages, pass the struct sgx_encl of > the enclave that is allocating the page. sgx_alloc_epc_page() will store > this value in the owner field of the struct sgx_epc_page. In a later > patch, VA pages will be placed in an unreclaimable queue, and then when the > cgroup max limit is reached and there are no more reclaimable pages and the > enclave must be OOM killed, all the VA pages associated with that enclave > can be uncharged and freed. > > To avoid casting needed to access the two types of owners: sgx_encl for VA > pages, sgx_encl_page for other pages, replace 'owner' field in sgx_epc_page > with a union of the two types. I think the action taken is correct but the reasoning is a bit convoluted. Why not instead put something like: "Because struct sgx_epc_page instances of VA pages are not owned by an sgx_encl_page instance in the first place, mark their owner as sgx_encl, in order to make it reachable from the unreclaimable list." The code change itself, and rest of the paragraphs do look reasonable. BR, Jarkko
