Re: [PATCH 2/4] KVM: s390: Remove cond_resched() calls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] Missing cpu_relax() in busy-wait loops in ipte_lock_simple and ipte_lock_siif.
--

--- Patch [2]: [PATCH 2/4] KVM: s390: Remove cond_resched() calls ---
commit 85d6ff7bc8ba9ff52b5d710af6d19c877a1f970f
Author: Heiko Carstens <hca@linux.ibm.com>

KVM: s390: Remove cond_resched() calls
    
Since [1] cond_resched() is a no-op on s390. Remove all calls.

[ ... ]

> diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> index 36102b2727fbf..0d389a302a70e 100644
> --- a/arch/s390/kvm/gaccess.c
> +++ b/arch/s390/kvm/gaccess.c
> @@ -151,10 +151,8 @@ static void ipte_lock_simple(struct kvm *kvm)
>  	ic = &kvm->arch.sca->ipte_control;
>  	old = READ_ONCE(*ic);
>  	do {
> -		if (old.k) {
> -			cond_resched();
> +		if (old.k)
>  			goto retry;
> -		}

[Severity: Medium]
This isn't a bug introduced by this patch, but removing cond_resched() makes
it clear that this becomes a tight read-spin loop when old.k evaluates to true.

Since cond_resched() is a no-op, this loop continuously re-reads memory without
yielding or pausing.

Should this include a cpu_relax() to prevent saturating memory bandwidth
and starving other SMT threads on the same core?

>  		new = old;
>  		new.k = 1;
>  	} while (!try_cmpxchg(&ic->val, &old.val, new.val));
> @@ -189,10 +187,8 @@ static void ipte_lock_siif(struct kvm *kvm)
>  	ic = &kvm->arch.sca->ipte_control;
>  	old = READ_ONCE(*ic);
>  	do {
> -		if (old.kg) {
> -			cond_resched();
> +		if (old.kg)
>  			goto retry;
> -		}

[Severity: Medium]
This isn't a bug introduced by this patch, but similar to ipte_lock_simple(),
this creates a tight read-spin loop when old.kg evaluates to true.

Would adding cpu_relax() be appropriate here as well?

>  		new = old;
>  		new.k = 1;
>  		new.kh++;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727162430.1599396-1-hca@linux.ibm.com?part=2