Re: [PATCH v5 11/18] x86/sgx: store unreclaimable pages in LRU lists
On Tue, 2023-10-03 at 00:15 -0500, Haitao Huang wrote:
> On Thu, 28 Sep 2023 04:41:33 -0500, Huang, Kai wrote:
>
> >
> > > --- a/arch/x86/kernel/cpu/sgx/encl.c
> > > +++ b/arch/x86/kernel/cpu/sgx/encl.c
> > > @@ -746,6 +746,7 @@ void sgx_encl_release(struct kref *ref)
> > > xa_destroy(>page_array);
> > >
> > > if (!encl->secs_child_cnt && encl->secs.epc_page) {
> > > + sgx_drop_epc_page(encl->secs.epc_page);
> > > sgx_encl_free_epc_page(encl->secs.epc_page);
> > > encl->secs.epc_page = NULL;
> > > }
> >
> > The "record" of SECS/VA pages should be done together with this. I see
> > no
> > reason why the "record" and "drop" are separated into different patches.
>
> "record" of SECS/VA pages are done in this patch. Before nothing done in
> "record" for them because no tracking LRU lists for them. Now they are
> tracked.
>
>
I was talking about calling sgx_record_epc_page() for SECS/VA:
@@ -113,6 +113,9 @@ static int sgx_encl_create(struct sgx_encl *encl, struct
sgx_secs *secs)
encl->attributes = secs->attributes;
encl->attributes_mask = SGX_ATTR_UNPRIV_MASK;
+ sgx_record_epc_page(encl->secs.epc_page,
+ SGX_EPC_PAGE_UNRECLAIMABLE);
This piece of code *literally* does the record.
Re: [PATCH v5 11/18] x86/sgx: store unreclaimable pages in LRU lists
On Wed, 27 Sep 2023 06:57:18 -0500, Huang, Kai wrote: On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote: From: Sean Christopherson When an OOM event occurs, all pages associated with an enclave will need to be freed, including pages that are not currently tracked by the cgroup LRU lists. What are "cgroup LRU lists"? Will reword it. At them moment there is only one global sgx_epc_lru_lists. Add a new "unreclaimable" list to the sgx_epc_lru_lists struct and update the "sgx_record/drop_epc_pages()" functions for adding/removing VA and SECS pages to/from this "unreclaimable" list. Sorry I don't follow the logic between the two paragraphs. What is the exact problem? How does the new "unreclaimable" list solve the problem? Currently they are not tracked in a list managed by the ksgxd (future cgroup worker). So add a list to track them. Thanks Haitao
Re: [PATCH v5 11/18] x86/sgx: store unreclaimable pages in LRU lists
On Thu, 28 Sep 2023 04:41:33 -0500, Huang, Kai wrote:
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -746,6 +746,7 @@ void sgx_encl_release(struct kref *ref)
xa_destroy(>page_array);
if (!encl->secs_child_cnt && encl->secs.epc_page) {
+ sgx_drop_epc_page(encl->secs.epc_page);
sgx_encl_free_epc_page(encl->secs.epc_page);
encl->secs.epc_page = NULL;
}
The "record" of SECS/VA pages should be done together with this. I see
no
reason why the "record" and "drop" are separated into different patches.
"record" of SECS/VA pages are done in this patch. Before nothing done in
"record" for them because no tracking LRU lists for them. Now they are
tracked.
Thanks
Haitao
Re: [PATCH v5 11/18] x86/sgx: store unreclaimable pages in LRU lists
> --- a/arch/x86/kernel/cpu/sgx/encl.c
> +++ b/arch/x86/kernel/cpu/sgx/encl.c
> @@ -746,6 +746,7 @@ void sgx_encl_release(struct kref *ref)
> xa_destroy(>page_array);
>
> if (!encl->secs_child_cnt && encl->secs.epc_page) {
> + sgx_drop_epc_page(encl->secs.epc_page);
> sgx_encl_free_epc_page(encl->secs.epc_page);
> encl->secs.epc_page = NULL;
> }
The "record" of SECS/VA pages should be done together with this. I see no
reason why the "record" and "drop" are separated into different patches.
Re: [PATCH v5 11/18] x86/sgx: store unreclaimable pages in LRU lists
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote: > From: Sean Christopherson > > When an OOM event occurs, all pages associated with an enclave will need > to be freed, including pages that are not currently tracked by the > cgroup LRU lists. What are "cgroup LRU lists"? > > Add a new "unreclaimable" list to the sgx_epc_lru_lists struct and > update the "sgx_record/drop_epc_pages()" functions for adding/removing > VA and SECS pages to/from this "unreclaimable" list. Sorry I don't follow the logic between the two paragraphs. What is the exact problem? How does the new "unreclaimable" list solve the problem?
