Re: [RFC PATCH v2 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup
From: Dave Hansen
Date: Fri Apr 26 2024 - 13:07:16 EST
On 4/26/24 07:18, Bojun Zhu wrote:
> for (c = 0 ; c < modp->length; c += PAGE_SIZE) {
> + if (sgx_check_signal_and_resched()) {
> + if (!c)
> + ret = -ERESTARTSYS;
> +
> + goto out;
> + }
This construct is rather fugly. Let's not perpetuate it, please. Why
not do:
int ret = -ERESTARTSYS;
...
for (c = 0 ; c < modp->length; c += PAGE_SIZE) {
if (sgx_check_signal_and_resched())
goto out;
Then, voila, when c==0 on the first run through the loop, you'll get a
ret=-ERESTARTSYS.
But honestly, it seems kinda silly to annotate all these loops with
explicit cond_resched()s. I'd much rather do this once and, for
instance, just wrap the enclave locks:
- mutex_lock(&encl->lock);
+ sgx_lock_enclave(encl);
and then have the lock function do the rescheds. I assume that
mutex_lock() isn't doing this generically for performance reasons. But
we don't care in SGX land and can just resched to our heart's content.