Re: [PATCH v2 01/18] x86/sgx: Call cond_resched() at the end of sgx_reclaim_pages()

From: Dave Hansen
Date: Fri Dec 02 2022 - 16:33:30 EST


On 12/2/22 10:36, Kristen Carlson Accardi wrote:
> From: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
>
> In order to avoid repetition of cond_resched() in ksgxd() and
> sgx_alloc_epc_page(), move the invocation of post-reclaim cond_resched()
> inside sgx_reclaim_pages(). Except in the case of sgx_reclaim_direct(),
> sgx_reclaim_pages() is always called in a loop and is always followed
> by a call to cond_resched(). This will hold true for the EPC cgroup
> as well, which adds even more calls to sgx_reclaim_pages() and thus
> cond_resched(). Calls to sgx_reclaim_direct() may be performance
> sensitive. Allow sgx_reclaim_direct() to avoid the cond_resched()
> call by moving the original sgx_reclaim_pages() call to
> __sgx_reclaim_pages() and then have sgx_reclaim_pages() become a
> wrapper around that call with a cond_resched().
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Signed-off-by: Kristen Carlson Accardi <kristen@xxxxxxxxxxxxxxx>
> Cc: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> arch/x86/kernel/cpu/sgx/main.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> index 160c8dbee0ab..ffce6fc70a1f 100644
> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -287,7 +287,7 @@ static void sgx_reclaimer_write(struct sgx_epc_page *epc_page,
> * problematic as it would increase the lock contention too much, which would
> * halt forward progress.
> */
> -static void sgx_reclaim_pages(void)
> +static void __sgx_reclaim_pages(void)
> {
> struct sgx_epc_page *chunk[SGX_NR_TO_SCAN];
> struct sgx_backing backing[SGX_NR_TO_SCAN];
> @@ -369,6 +369,12 @@ static void sgx_reclaim_pages(void)
> }
> }
>
> +static void sgx_reclaim_pages(void)
> +{
> + __sgx_reclaim_pages();
> + cond_resched();
> +}

Why bother with the wrapper? Can't we just put cond_resched() in the
existing sgx_reclaim_pages()?