On Fri, 16 Feb 2024 09:15:59 -0600, Dave Hansen <dave.hansen@xxxxxxxxx>
wrote:
On 2/5/24 13:06, Haitao Huang wrote:
@@ -414,7 +416,7 @@ static void sgx_reclaim_pages_global(void)
void sgx_reclaim_direct(void)
{
if (sgx_should_reclaim(SGX_NR_LOW_PAGES))
- sgx_reclaim_pages_global();
+ sgx_reclaim_pages_global(false);
}
static int ksgxd(void *p)
@@ -437,7 +439,7 @@ static int ksgxd(void *p)
sgx_should_reclaim(SGX_NR_HIGH_PAGES));
if (sgx_should_reclaim(SGX_NR_HIGH_PAGES))
- sgx_reclaim_pages_global();
+ sgx_reclaim_pages_global(true);
cond_resched();
}
First, I'm never a fan of random true/false or 0/1 arguments to
functions like this. You end up having to go look at the called
function to make any sense of it. You can either do an enum, or some
construct like this:
if (sgx_should_reclaim(SGX_NR_HIGH_PAGES)) {
bool indirect = true;
sgx_reclaim_pages_global(indirect);
}
Yeah, it takes a few more lines but it saves you having to comment the
thing.
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().
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.
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?
Thanks
Haitao
[1]
https://lore.kernel.org/linux-sgx/9c269c70-35fe-a1a4-34c9-b1e62ab3bb3b@xxxxxxxxx/
