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

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Jul 30, 2026 at 01:34:31PM +0200, Holger Dengler wrote:
> 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?

Yes, guest migration is very likely far more disruptive than most
anything else. Perhaps clarify the code comment to include some of this
explanation?