On Fri, 16 Feb 2024 15:55:10 -0600, Dave Hansen <dave.hansen@xxxxxxxxx>
wrote:
On 2/16/24 13:38, Haitao Huang wrote:
On Fri, 16 Feb 2024 09:15:59 -0600, Dave Hansen <dave.hansen@xxxxxxxxx>
wrote:
...
Does this 'indirect' change any behavior other than whether it does a
search for an mm to find a place to charge the backing storage?
No.
Instead of passing a flag around, why not just pass the mm?
There is no need to pass in mm. We could just check if current->mm ==
NULL for the need of doing the search in the enclave mm list.
But you had a concern [1] that the purpose was not clear hence suggested
current_is_ksgxd().
Right, because there was only one possible way that mm could be NULL but
it wasn't obvious from the code what that way was.
Would it be OK if we replace current_is_ksgxd() with (current->flags &
PF_KTHREAD)? That would express the real intent of checking if calling
context is not in a user context.
No, I think that focuses on the symptom and not on the fundamental
problem.
The fundamental problem is that you need an mm in order to charge your
allocations to the right group. Indirect reclaim means you are not in a
context which is connected to the mm that should be charged while direct
reclaim is.
This refactoring out of 'indirect' or passing the mm around really
wants
to be in its own patch anyway.
Looks like I could do:
1) refactoring of 'indirect' value/enum suggested above. This seems the
most straightforward without depending on any assumptions of other
kernel code.
2) replace current_is_ksgxd() with current->mm == NULL. This assumes
kthreads has no mm.
3) replace current_is_ksgxd() with current->flags & PF_KTHREAD. This is
direct use of the flag PF_KTHREAD, so it should be better than #2?
Any preference or further thoughts?
Pass around a:
struct mm_struct *charge_mm
Then, at the bottom do:
/*
* Backing RAM allocations need to be charged to some mm and
* associated cgroup. If this context does not have an mm to
* charge, search the enclave's mm_list to find some mm
* associated with this enclave.
*/
if (!charge_mm)
... do slow mm lookup
else
return mm_to_cgroup_whatever(charge_mm);
Then just comment the call sites where the initial charge_mm comes in:
/* Indirect SGX reclaim, no mm to charge, so NULL: */
foo(..., NULL);
/* Direct SGX reclaim, charge current mm for allocations: */
foo(..., current->mm);
Okay. got it now.
Thank you very much!
Haitao
