| Message ID | 20230913040635.28815-4-haitao.huang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Cgroup support for SGX EPC memory | expand |
On Wed Sep 13, 2023 at 7:06 AM EEST, Haitao Huang wrote: > From: Kristen Carlson Accardi <kristen@linux.intel.com> > > Introduce a data structure to wrap the existing reclaimable list and its > spinlock. Each cgroup later will have one instance of this structure to > track EPC pages allocated for processes associated with the same cgroup. > Just like the global SGX reclaimer (ksgxd), an EPC cgroup reclaims pages > from the reclaimable list in this structure when its usage reaches near > its limit. > > Currently, ksgxd does not track the VA, SECS pages. They are considered > as 'unreclaimable' pages that are only deallocated when their respective > owning enclaves are destroyed and all associated resources released. > > When an EPC cgroup can not reclaim any more reclaimable EPC pages to > reduce its usage below its limit, the cgroup must also reclaim those > unreclaimables by killing their owning enclaves. The VA and SECS pages > later are also tracked in an 'unreclaimable' list added to this structure > to support this OOM killing of enclaves. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com> > Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> > Cc: Sean Christopherson <seanjc@google.com> > --- > V4: > - Removed unneeded comments for the spinlock and the non-reclaimables. > (Kai, Jarkko) > - Revised the commit to add introduction comments for unreclaimables and > multiple LRU lists.(Kai) > - Reordered the patches: delay all changes for unreclaimables to > later, and this one becomes the first change in the SGX subsystem. > > V3: > - Removed the helper functions and revised commit messages. > --- > arch/x86/kernel/cpu/sgx/sgx.h | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h > index d2dad21259a8..018414b2abe8 100644 > --- a/arch/x86/kernel/cpu/sgx/sgx.h > +++ b/arch/x86/kernel/cpu/sgx/sgx.h > @@ -83,6 +83,20 @@ static inline void *sgx_get_epc_virt_addr(struct sgx_epc_page *page) > return section->virt_addr + index * PAGE_SIZE; > } > > +/* > + * Tracks EPC pages reclaimable by the reclaimer (ksgxd). > + */ > +struct sgx_epc_lru_lists { > + spinlock_t lock; > + struct list_head reclaimable; > +}; > + > +static inline void sgx_lru_init(struct sgx_epc_lru_lists *lrus) > +{ > + spin_lock_init(&lrus->lock); > + INIT_LIST_HEAD(&lrus->reclaimable); > +} > + > struct sgx_epc_page *__sgx_alloc_epc_page(void); > void sgx_free_epc_page(struct sgx_epc_page *page); > > -- > 2.25.1 > Looks good but not yet time for ack'ing. BR, Jarkko
Some non-technical staff: On Tue, 2023-09-12 at 21:06 -0700, Haitao Huang wrote: > From: Kristen Carlson Accardi <kristen@linux.intel.com> The patch was from Kristen, but ... > > Introduce a data structure to wrap the existing reclaimable list and its > spinlock. Each cgroup later will have one instance of this structure to > track EPC pages allocated for processes associated with the same cgroup. > Just like the global SGX reclaimer (ksgxd), an EPC cgroup reclaims pages > from the reclaimable list in this structure when its usage reaches near > its limit. > > Currently, ksgxd does not track the VA, SECS pages. They are considered > as 'unreclaimable' pages that are only deallocated when their respective > owning enclaves are destroyed and all associated resources released. > > When an EPC cgroup can not reclaim any more reclaimable EPC pages to > reduce its usage below its limit, the cgroup must also reclaim those > unreclaimables by killing their owning enclaves. The VA and SECS pages > later are also tracked in an 'unreclaimable' list added to this structure > to support this OOM killing of enclaves. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com> ... it was firstly signed by Sean and then Kristen, which doesn't sound right. If the patch was from Kristen, then either Sean's SoB should come after Kristen's (which means Sean took Kristen's patch and signed it), or you need to have a Co-developed-by tag for Sean right before his SoB (which indicates Sean participated in the development of the patch but likely he wasn't the main developer). But I _guess_ the patch was just from Sean. > Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> > Cc: Sean Christopherson <seanjc@google.com> You don't need 'Cc:' Sean if the patch has Sean's SoB. More information please refer to "When to use Acked-by:, Cc:, and Co-developed- by" section here: https://docs.kernel.org/process/submitting-patches.html Also an explanation of when to use 'Cc:' from Sean (ignore technical staff): https://lore.kernel.org/lkml/ZOZteOxJvq9v609G@google.com/ (And please check other patches too.)
On 9/14/23 03:31, Huang, Kai wrote: >> Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> >> Cc: Sean Christopherson <seanjc@google.com> > You don't need 'Cc:' Sean if the patch has Sean's SoB. It is a SoB for Sean's @intel address and cc's his @google address. It is fine.
On Thu, 2023-09-14 at 09:13 -0700, Dave Hansen wrote: > On 9/14/23 03:31, Huang, Kai wrote: > > > Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> > > > Cc: Sean Christopherson <seanjc@google.com> > > You don't need 'Cc:' Sean if the patch has Sean's SoB. > > It is a SoB for Sean's @intel address and cc's his @google address. > > It is fine. Oops I didn't notice the email difference. Thanks for pointing out!
On Thu, 14 Sep 2023 05:31:30 -0500, Huang, Kai <kai.huang@intel.com> wrote: > Some non-technical staff: > > On Tue, 2023-09-12 at 21:06 -0700, Haitao Huang wrote: >> From: Kristen Carlson Accardi <kristen@linux.intel.com> > > The patch was from Kristen, but ... > >> >> Introduce a data structure to wrap the existing reclaimable list and its >> spinlock. Each cgroup later will have one instance of this structure to >> track EPC pages allocated for processes associated with the same cgroup. >> Just like the global SGX reclaimer (ksgxd), an EPC cgroup reclaims pages >> from the reclaimable list in this structure when its usage reaches near >> its limit. >> >> Currently, ksgxd does not track the VA, SECS pages. They are considered >> as 'unreclaimable' pages that are only deallocated when their respective >> owning enclaves are destroyed and all associated resources released. >> >> When an EPC cgroup can not reclaim any more reclaimable EPC pages to >> reduce its usage below its limit, the cgroup must also reclaim those >> unreclaimables by killing their owning enclaves. The VA and SECS pages >> later are also tracked in an 'unreclaimable' list added to this >> structure >> to support this OOM killing of enclaves. >> >> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> >> Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com> > > ... it was firstly signed by Sean and then Kristen, which doesn't sound > right. > > If the patch was from Kristen, then either Sean's SoB should come after > Kristen's (which means Sean took Kristen's patch and signed it), or you > need to > have a Co-developed-by tag for Sean right before his SoB (which > indicates Sean > participated in the development of the patch but likely he wasn't the > main > developer). > > But I _guess_ the patch was just from Sean. > From what I see: In v1 kristen included a "From" tsg for Sean. In v2 she split the original patch into two and added some wrappers/ At that time, she removed the "From" tag for both patches but kept the SOB and CC. @Kristen, could you confirm? I only removed the wrappers from v2 based on Dave's comments. So if confirmed by Kristen, should we add "From" tag for Sean? I'll double check the other patches. Thanks Haitao
diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h index d2dad21259a8..018414b2abe8 100644 --- a/arch/x86/kernel/cpu/sgx/sgx.h +++ b/arch/x86/kernel/cpu/sgx/sgx.h @@ -83,6 +83,20 @@ static inline void *sgx_get_epc_virt_addr(struct sgx_epc_page *page) return section->virt_addr + index * PAGE_SIZE; } +/* + * Tracks EPC pages reclaimable by the reclaimer (ksgxd). + */ +struct sgx_epc_lru_lists { + spinlock_t lock; + struct list_head reclaimable; +}; + +static inline void sgx_lru_init(struct sgx_epc_lru_lists *lrus) +{ + spin_lock_init(&lrus->lock); + INIT_LIST_HEAD(&lrus->reclaimable); +} + struct sgx_epc_page *__sgx_alloc_epc_page(void); void sgx_free_epc_page(struct sgx_epc_page *page);