Re: [PATCH v12 07/14] x86/sgx: Abstract tracking reclaimable pages in LRU

From: Haitao Huang
Date: Tue Apr 16 2024 - 18:51:09 EST


On Tue, 16 Apr 2024 09:07:33 -0500, Huang, Kai <kai.huang@xxxxxxxxx> wrote:

On Mon, 2024-04-15 at 20:20 -0700, Haitao Huang wrote:
From: Kristen Carlson Accardi <kristen@xxxxxxxxxxxxxxx>

The functions, sgx_{mark,unmark}_page_reclaimable(), manage the tracking
of reclaimable EPC pages: sgx_mark_page_reclaimable() adds a newly
allocated page into the global LRU list while
sgx_unmark_page_reclaimable() does the opposite. Abstract the hard coded
global LRU references in these functions to make them reusable when
pages are tracked in per-cgroup LRUs.

Create a helper, sgx_lru_list(), that returns the LRU that tracks a given
EPC page. It simply returns the global LRU now, and will later return
the LRU of the cgroup within which the EPC page was allocated. Replace
the hard coded global LRU with a call to this helper.

Next patches will first get the cgroup reclamation flow ready while
keeping pages tracked in the global LRU and reclaimed by ksgxd before we
make the switch in the end for sgx_lru_list() to return per-cgroup
LRU.

I found the first paragraph hard to read. Provide my version below for
your reference:

"
The SGX driver tracks reclaimable EPC pages via
sgx_mark_page_reclaimable(), which adds the newly allocated page into the
global LRU list. sgx_unmark_page_reclaimable() does the opposite.

To support SGX EPC cgroup, the SGX driver will need to maintain an LRU
list for each cgroup, and the new allocated EPC page will need to be added
to the LRU of associated cgroup, but not always the global LRU list.

When sgx_mark_page_reclaimable() is called, the cgroup that the new
allocated EPC page belongs to is already known, i.e., it has been set to
the 'struct sgx_epc_page'.

Add a helper, sgx_lru_list(), to return the LRU that the EPC page should
be/is added to for the given EPC page. Currently it just returns the
global LRU. Change sgx_{mark|unmark}_page_reclaimable() to use the helper
function to get the LRU from the EPC page instead of referring to the
global LRU directly.

This allows EPC page being able to be tracked in "per-cgroup" LRU when
that becomes ready.
"

Thanks. Will use

Nit:

That being said, is sgx_epc_page_lru() better than sgx_lru_list()?


Sure


Co-developed-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
Signed-off-by: Kristen Carlson Accardi <kristen@xxxxxxxxxxxxxxx>
Co-developed-by: Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx>
Signed-off-by: Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx>
Reviewed-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
Tested-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
---


Feel free to add:

Reviewed-by: Kai Huang <kai.huang@xxxxxxxxx>

Thanks
Haitao