/*
@@ -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.

Reply via email to