Re: [PATCH v3 1/4] s390/crypto: Replace cond_resched() with msleep(1)

[Holger Dengler <dengler@linux.ibm.com>]

Peter,

On 7/30/26 12:11, Peter Zijlstra wrote:
> On Thu, Jul 30, 2026 at 07:29:04AM +0200, Heiko Carstens wrote:
>> With [1] cond_resched() is always compiled away and becomes a no-op.
>>
>> The comments for all cond_resched() calls in crypto code however indicate
>> that the current process should be scheduled away to avoid instant
>> re-invocation of a callback. This is not what cond_resched() would do or
>> did.
>>
>> Instead of just removing the cond_resched() calls, replace them with
>> msleep() calls, as suggested by Holger Dengler. This forces the current
>> task to be scheduled away (sleeps) like originally intended.
>>
>> [1] commit 7dadeaa6e851 ("sched: Further restrict the preemption modes")
>>
>> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
>> ---
>>  arch/s390/crypto/paes_s390.c  | 8 ++++----
>>  arch/s390/crypto/phmac_s390.c | 4 ++--
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
>> index 8cfe6166c193..511cb6105436 100644
>> --- a/arch/s390/crypto/paes_s390.c
>> +++ b/arch/s390/crypto/paes_s390.c
>> @@ -555,7 +555,7 @@ static int ecb_paes_do_one_request(struct crypto_engine *engine, void *areq)
>>  		 * To avoid immediately re-invocation of this callback,
>>  		 * tell the scheduler to voluntarily give up the CPU here.
>>  		 */
>> -		cond_resched();
>> +		msleep(1);
>>  		pr_debug("rescheduling request\n");
>>  		return -ENOSPC;
>>  	} else if (rc) {
> 
> I am somewhat conflicted on this. It will add a 'random' delay to this
> crypto user (which might be real-time task) that is not related to the
> actual event this is waiting for.
> 
> That is, it could be that this key expiration thing is sorted way faster
> than this one milisecond.
> 
> Is there nothing the crypto layer can do that is more clever; like a
> condition variable on the key update when -ENOSPC is returned or
> something.

Let me give a bit of background here: The protected key can only get
invalid, if the linux instance (z/VM or KVM guest) is moved to another
hypervisor on a different machine (aka life guest relocation). In such a
case, the crypto accelerator card and the host has to exchange the "real
key", which is wrapped by the host and handed back to the guest as the
re-newed protected key. Unfortunately there is no asynchronous trigger
on completion, you have to re-try (and maybe get another "in progress"
return).

And as if that weren't bad enough, if this key exchange between card and
host is the first one, card and host has to instanciate a secure
communication channel (including a key exchange for the transport layer).

I agree, this sounds rally bad for real-time tasks. But we're talking
about 2nd-level virtualization (with non-real-time hypervisors below)
and about cases, which can only happen right after a guest relocation to
another machine. Would the current solution be acceptable under these
circumstances?

> This all sounds like a horrible hack one way or the other.

Ähh, yes... :)

-- 
Mit freundlichen Grüßen / Kind regards
Holger Dengler