Messages in this thread | | | Date | Fri, 26 Apr 2024 10:06:56 -0700 | Subject | Re: [RFC PATCH v2 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup | From | Dave Hansen <> |
| |
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.
| |