On Mon, 29 Apr 2024 17:18:05 -0500, Huang, Kai <kai.huang@xxxxxxxxx> wrote:
/*
@@ -42,7 +63,8 @@ static inline struct sgx_epc_lru_list
*sgx_lru_list(struct sgx_epc_page *epc_pag
*/
static inline bool sgx_can_reclaim(void)
{
- return !list_empty(&sgx_global_lru.reclaimable);
+ return !sgx_cgroup_lru_empty(misc_cg_root()) ||
+ !list_empty(&sgx_global_lru.reclaimable);
}
Shouldn't this be:
if (IS_ENABLED(CONFIG_CGROUP_MISC))
return !sgx_cgroup_lru_empty(misc_cg_root());
else
return !list_empty(&sgx_global_lru.reclaimable);
?
In this way, it is consistent with the sgx_reclaim_pages_global()
below.
I changed to this way because sgx_cgroup_lru_empty() is now defined in
both KConfig cases.
And it seems better to minimize use of the KConfig variables based on
earlier feedback (some are yours).
Don't really have strong preference here. So let me know one way of the
other.
But IMHO your code could be confusing, e.g., it can be interpreted as:
The EPC pages can be managed by both the cgroup LRUs and the
sgx_global_lru simultaneously at runtime when CONFIG_CGROUP_MISC
is on.
Which is not true.
So we should make the code clearly reflect the true behaviour.
Ok, I'll change back.
Thanks
Haitao
