lkml.org 
[lkml]   [2024]   [Apr]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v12 12/14] x86/sgx: Turn on per-cgroup EPC reclamation
From

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

\
 
 \ /
  Last update: 2024-05-27 18:08    [W:0.049 / U:0.520 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site