* [PATCH 0/4] s390: Remove or replace cond_resched() calls
@ 2026-07-27 16:24 Heiko Carstens
2026-07-27 16:24 ` [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule() Heiko Carstens
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Heiko Carstens @ 2026-07-27 16:24 UTC (permalink / raw)
To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Harald Freudenberger, Holger Dengler,
Vineeth Vijayan, Peter Oberparleiter, Janosch Frank,
Claudio Imbrenda, David Hildenbrand, Peter Zijlstra
Cc: linux-s390, linux-kernel
Since [1] it is only possible to select the PREEMPT or PREEMPT_LAZY
preemption models for s390. This has caused some minor problems, but
everything has been addressed.
Given that for both preemption models cond_resched() is a no-op remove
or replace it everywhere.
[1] commit 7dadeaa6e851 ("sched: Further restrict the preemption modes")
Heiko Carstens (4):
s390/crypto: Replace cond_resched() with schedule()
KVM: s390: Remove cond_resched() calls
s390: Remove cond_resched() calls
s390/cio: Remove cond_resched() calls
arch/s390/crypto/paes_s390.c | 8 ++++----
arch/s390/crypto/phmac_s390.c | 4 ++--
arch/s390/include/asm/uv.h | 1 -
arch/s390/kernel/perf_cpum_cf.c | 1 -
arch/s390/kvm/gaccess.c | 8 ++------
arch/s390/kvm/gmap.c | 4 ----
arch/s390/kvm/kvm-s390.c | 1 -
arch/s390/kvm/pv.c | 1 -
arch/s390/kvm/vsie.c | 1 -
arch/s390/mm/cmm.c | 2 --
arch/s390/mm/gmap_helpers.c | 1 -
arch/s390/mm/pageattr.c | 5 -----
arch/s390/mm/pgalloc.c | 1 -
arch/s390/pci/pci.c | 4 +---
drivers/s390/cio/css.c | 8 --------
15 files changed, 9 insertions(+), 41 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule()
2026-07-27 16:24 [PATCH 0/4] s390: Remove or replace cond_resched() calls Heiko Carstens
@ 2026-07-27 16:24 ` Heiko Carstens
2026-07-27 17:05 ` sashiko-bot
2026-07-28 7:05 ` Harald Freudenberger
2026-07-27 16:24 ` [PATCH 2/4] KVM: s390: Remove cond_resched() calls Heiko Carstens
` (2 subsequent siblings)
3 siblings, 2 replies; 16+ messages in thread
From: Heiko Carstens @ 2026-07-27 16:24 UTC (permalink / raw)
To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Harald Freudenberger, Holger Dengler,
Vineeth Vijayan, Peter Oberparleiter, Janosch Frank,
Claudio Imbrenda, David Hildenbrand, Peter Zijlstra
Cc: linux-s390, linux-kernel
The comment for all cond_resched() calls in crypto code says that the
process should be scheduled away to avoid instant re-invocation of a
callback. This is not what cond_resched() would do or did.
Replace cond_resched() with a call to schedule() which comes closer, but
it looks more like this code wants an msleep() call.
Note that with [1] cond_resched() is always compiled away and becomes a
no-op.
[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..d02de945f864 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();
+ schedule();
pr_debug("rescheduling request\n");
return -ENOSPC;
} else if (rc) {
@@ -821,7 +821,7 @@ static int cbc_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();
+ schedule();
pr_debug("rescheduling request\n");
return -ENOSPC;
} else if (rc) {
@@ -1129,7 +1129,7 @@ static int ctr_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();
+ schedule();
pr_debug("rescheduling request\n");
return -ENOSPC;
} else if (rc) {
@@ -1572,7 +1572,7 @@ static int xts_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();
+ schedule();
pr_debug("rescheduling request\n");
return -ENOSPC;
} else if (rc) {
diff --git a/arch/s390/crypto/phmac_s390.c b/arch/s390/crypto/phmac_s390.c
index 03ca33ffe6cc..bddfdcee6cb3 100644
--- a/arch/s390/crypto/phmac_s390.c
+++ b/arch/s390/crypto/phmac_s390.c
@@ -895,7 +895,7 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
* tell scheduler to voluntarily give up the CPU here.
*/
pr_debug("rescheduling request\n");
- cond_resched();
+ schedule();
return -ENOSPC;
} else if (rc) {
hwh_advance(hwh, rc);
@@ -916,7 +916,7 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq)
* tell scheduler to voluntarily give up the CPU here.
*/
pr_debug("rescheduling request\n");
- cond_resched();
+ schedule();
return -ENOSPC;
}
break;
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/4] KVM: s390: Remove cond_resched() calls
2026-07-27 16:24 [PATCH 0/4] s390: Remove or replace cond_resched() calls Heiko Carstens
2026-07-27 16:24 ` [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule() Heiko Carstens
@ 2026-07-27 16:24 ` Heiko Carstens
2026-07-27 16:57 ` sashiko-bot
2026-07-27 16:24 ` [PATCH 3/4] " Heiko Carstens
2026-07-27 16:24 ` [PATCH 4/4] s390/cio: " Heiko Carstens
3 siblings, 1 reply; 16+ messages in thread
From: Heiko Carstens @ 2026-07-27 16:24 UTC (permalink / raw)
To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Harald Freudenberger, Holger Dengler,
Vineeth Vijayan, Peter Oberparleiter, Janosch Frank,
Claudio Imbrenda, David Hildenbrand, Peter Zijlstra
Cc: linux-s390, linux-kernel
Since [1] cond_resched() is a no-op on s390. Remove all calls.
[1] commit 7dadeaa6e851 ("sched: Further restrict the preemption modes")
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
arch/s390/include/asm/uv.h | 1 -
arch/s390/kvm/gaccess.c | 8 ++------
arch/s390/kvm/gmap.c | 4 ----
arch/s390/kvm/kvm-s390.c | 1 -
arch/s390/kvm/pv.c | 1 -
arch/s390/kvm/vsie.c | 1 -
arch/s390/mm/gmap_helpers.c | 1 -
7 files changed, 2 insertions(+), 15 deletions(-)
diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index d919e69662f5..795da130342a 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -461,7 +461,6 @@ static inline int uv_call_sched(unsigned long r1, unsigned long r2)
do {
cc = __uv_call(r1, r2);
- cond_resched();
} while (cc > 1);
return cc;
}
diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 36102b2727fb..0d389a302a70 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;
- }
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;
- }
new = old;
new.k = 1;
new.kh++;
diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
index 8abb4f55b306..3758e4009709 100644
--- a/arch/s390/kvm/gmap.c
+++ b/arch/s390/kvm/gmap.c
@@ -941,7 +941,6 @@ void gmap_split_huge_pages(struct gmap *gmap)
scoped_guard(read_lock, &gmap->kvm->mmu_lock)
start = _dat_walk_gfn_range(start, asce_end(gmap->asce), gmap->asce,
&ops, DAT_WALK_IGN_HOLES, gmap);
- cond_resched();
} while (start);
}
@@ -963,7 +962,6 @@ static int _gmap_enable_skeys(struct gmap *gmap)
do {
scoped_guard(write_lock, &gmap->kvm->mmu_lock)
start = dat_reset_skeys(gmap->asce, start);
- cond_resched();
} while (start);
return 0;
}
@@ -1019,7 +1017,6 @@ int gmap_pv_destroy_range(struct gmap *gmap, gfn_t start, gfn_t end, bool interr
DAT_WALK_IGN_HOLES, NULL);
if (interruptible && fatal_signal_pending(current))
return -EINTR;
- cond_resched();
} while (start && start < end);
return 0;
}
@@ -1138,7 +1135,6 @@ void _gmap_set_cmma_all(struct gmap *gmap, bool dirty)
gfn = _dat_walk_gfn_range(gfn, asce_end(gmap->asce), gmap->asce, &ops,
DAT_WALK_IGN_HOLES,
&gmap->kvm->arch.cmma_dirty_pages);
- cond_resched();
} while (gfn);
}
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 150b5dd2170e..2ede8f035a38 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1027,7 +1027,6 @@ static int kvm_s390_set_mem_control(struct kvm *kvm, struct kvm_device_attr *att
do {
scoped_guard(read_lock, &kvm->mmu_lock)
start_gfn = dat_reset_cmma(kvm->arch.gmap->asce, start_gfn);
- cond_resched();
} while (start_gfn);
ret = 0;
break;
diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
index 1beacc841ca8..935fe308a050 100644
--- a/arch/s390/kvm/pv.c
+++ b/arch/s390/kvm/pv.c
@@ -832,7 +832,6 @@ int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
while (offset < size) {
ret = unpack_one(kvm, addr, tweak, offset, rc, rrc);
if (ret == -EAGAIN) {
- cond_resched();
if (fatal_signal_pending(current))
break;
continue;
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index eea24562e7db..6dcd3b110c17 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -1434,7 +1434,6 @@ static int vsie_run(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
}
if (sg)
sg = gmap_put(sg);
- cond_resched();
}
if (sg)
sg = gmap_put(sg);
diff --git a/arch/s390/mm/gmap_helpers.c b/arch/s390/mm/gmap_helpers.c
index 4bf7c9012feb..d564cdefb90f 100644
--- a/arch/s390/mm/gmap_helpers.c
+++ b/arch/s390/mm/gmap_helpers.c
@@ -272,7 +272,6 @@ static int __gmap_helper_unshare_zeropages(struct mm_struct *mm)
* truncation. In that case, the shared zeropage would be gone
* and we can simply retry and make progress.
*/
- cond_resched();
goto retry;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/4] s390: Remove cond_resched() calls
2026-07-27 16:24 [PATCH 0/4] s390: Remove or replace cond_resched() calls Heiko Carstens
2026-07-27 16:24 ` [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule() Heiko Carstens
2026-07-27 16:24 ` [PATCH 2/4] KVM: s390: Remove cond_resched() calls Heiko Carstens
@ 2026-07-27 16:24 ` Heiko Carstens
2026-07-27 16:54 ` sashiko-bot
2026-07-27 16:24 ` [PATCH 4/4] s390/cio: " Heiko Carstens
3 siblings, 1 reply; 16+ messages in thread
From: Heiko Carstens @ 2026-07-27 16:24 UTC (permalink / raw)
To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Harald Freudenberger, Holger Dengler,
Vineeth Vijayan, Peter Oberparleiter, Janosch Frank,
Claudio Imbrenda, David Hildenbrand, Peter Zijlstra
Cc: linux-s390, linux-kernel
Since [1] cond_resched() is a no-op on s390. Remove all calls.
[1] commit 7dadeaa6e851 ("sched: Further restrict the preemption modes")
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
arch/s390/kernel/perf_cpum_cf.c | 1 -
arch/s390/mm/cmm.c | 2 --
arch/s390/mm/pageattr.c | 5 -----
arch/s390/mm/pgalloc.c | 1 -
arch/s390/pci/pci.c | 4 +---
5 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
index 2076ac22e2c4..5f2596be6c1e 100644
--- a/arch/s390/kernel/perf_cpum_cf.c
+++ b/arch/s390/kernel/perf_cpum_cf.c
@@ -1496,7 +1496,6 @@ static int cfset_all_copy(unsigned long arg, cpumask_t *mask)
goto out;
}
uptr += sizeof(struct s390_ctrset_cpudata) + cpuhw->used;
- cond_resched();
}
cpus = cpumask_weight(mask);
if (put_user(cpus, &ctrset_read->no_cpus))
diff --git a/arch/s390/mm/cmm.c b/arch/s390/mm/cmm.c
index eb7ef63fab1e..d342b68ee9be 100644
--- a/arch/s390/mm/cmm.c
+++ b/arch/s390/mm/cmm.c
@@ -95,7 +95,6 @@ static long cmm_alloc_pages(long nr, long *counter,
(*counter)++;
spin_unlock(&cmm_lock);
nr--;
- cond_resched();
}
return nr;
}
@@ -134,7 +133,6 @@ static long cmm_free_pages(long nr, long *counter, struct cmm_page_array **list)
inc = __cmm_free_pages(inc, counter, list);
if (inc)
break;
- cond_resched();
}
return nr + inc;
}
diff --git a/arch/s390/mm/pageattr.c b/arch/s390/mm/pageattr.c
index e6f788696dd1..1e202e3d08e7 100644
--- a/arch/s390/mm/pageattr.c
+++ b/arch/s390/mm/pageattr.c
@@ -105,7 +105,6 @@ static int walk_pte_level(pmd_t *pmdp, unsigned long addr, unsigned long end,
pgt_set((unsigned long *)ptep, pte_val(new), addr, CRDTE_DTT_PAGE);
ptep++;
addr += PAGE_SIZE;
- cond_resched();
} while (addr < end);
return 0;
}
@@ -194,7 +193,6 @@ static int walk_pmd_level(pud_t *pudp, unsigned long addr, unsigned long end,
}
pmdp++;
addr = next;
- cond_resched();
} while (addr < end);
return rc;
}
@@ -281,7 +279,6 @@ static int walk_pud_level(p4d_t *p4d, unsigned long addr, unsigned long end,
}
pudp++;
addr = next;
- cond_resched();
} while (addr < end && !rc);
return rc;
}
@@ -301,7 +298,6 @@ static int walk_p4d_level(pgd_t *pgd, unsigned long addr, unsigned long end,
rc = walk_pud_level(p4dp, addr, next, flags);
p4dp++;
addr = next;
- cond_resched();
} while (addr < end && !rc);
return rc;
}
@@ -323,7 +319,6 @@ static int change_page_attr(unsigned long addr, unsigned long end,
rc = walk_p4d_level(pgdp, addr, next, flags);
if (rc)
break;
- cond_resched();
} while (pgdp++, addr = next, addr < end && !rc);
return rc;
}
diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c
index 7ac44543e051..9610770fcf6d 100644
--- a/arch/s390/mm/pgalloc.c
+++ b/arch/s390/mm/pgalloc.c
@@ -268,7 +268,6 @@ static int base_segment_walk(unsigned long *origin, unsigned long addr,
return rc;
if (!alloc)
base_pgt_free(table);
- cond_resched();
} while (ste++, addr = next, addr < end);
return 0;
}
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 39bd2adfc240..2dec238f3f12 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -1173,10 +1173,8 @@ int zpci_scan_devices(void)
return rc;
zpci_add_devices(&scan_list);
- zpci_bus_for_each(zbus) {
+ zpci_bus_for_each(zbus)
zpci_bus_scan_bus(zbus);
- cond_resched();
- }
return 0;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/4] s390/cio: Remove cond_resched() calls
2026-07-27 16:24 [PATCH 0/4] s390: Remove or replace cond_resched() calls Heiko Carstens
` (2 preceding siblings ...)
2026-07-27 16:24 ` [PATCH 3/4] " Heiko Carstens
@ 2026-07-27 16:24 ` Heiko Carstens
2026-07-27 16:54 ` sashiko-bot
2026-07-28 9:28 ` Peter Oberparleiter
3 siblings, 2 replies; 16+ messages in thread
From: Heiko Carstens @ 2026-07-27 16:24 UTC (permalink / raw)
To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Harald Freudenberger, Holger Dengler,
Vineeth Vijayan, Peter Oberparleiter, Janosch Frank,
Claudio Imbrenda, David Hildenbrand, Peter Zijlstra
Cc: linux-s390, linux-kernel
Since [1] cond_resched() is a no-op on s390. Remove all calls.
[1] commit 7dadeaa6e851 ("sched: Further restrict the preemption modes")
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
drivers/s390/cio/css.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c
index e5a0ec6b4e3e..54b8b8639694 100644
--- a/drivers/s390/cio/css.c
+++ b/drivers/s390/cio/css.c
@@ -618,11 +618,6 @@ static int slow_eval_known_fn(struct subchannel *sch, void *data)
rc = css_evaluate_known_subchannel(sch, 1);
if (rc == -EAGAIN)
css_schedule_eval(sch->schid);
- /*
- * The loop might take long time for platforms with lots of
- * known devices. Allow scheduling here.
- */
- cond_resched();
}
return 0;
}
@@ -654,9 +649,6 @@ static int slow_eval_unknown_fn(struct subchannel_id schid, void *data)
default:
rc = 0;
}
- /* Allow scheduling here since the containing loop might
- * take a while. */
- cond_resched();
}
return rc;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] s390/cio: Remove cond_resched() calls
2026-07-27 16:24 ` [PATCH 4/4] s390/cio: " Heiko Carstens
@ 2026-07-27 16:54 ` sashiko-bot
2026-07-28 9:28 ` Peter Oberparleiter
1 sibling, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-07-27 16:54 UTC (permalink / raw)
To: Heiko Carstens
Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev
> Since [1] cond_resched() is a no-op on s390. Remove all calls.
>
> [1] commit 7dadeaa6e851 ("sched: Further restrict the preemption modes")
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260727162430.1599396-1-hca@linux.ibm.com?part=4
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] s390: Remove cond_resched() calls
2026-07-27 16:24 ` [PATCH 3/4] " Heiko Carstens
@ 2026-07-27 16:54 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-07-27 16:54 UTC (permalink / raw)
To: Heiko Carstens
Cc: linux-s390, Vasily Gorbik, Heiko Carstens, Alexander Gordeev
> Since [1] cond_resched() is a no-op on s390. Remove all calls.
>
> [1] commit 7dadeaa6e851 ("sched: Further restrict the preemption modes")
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260727162430.1599396-1-hca@linux.ibm.com?part=3
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] KVM: s390: Remove cond_resched() calls
2026-07-27 16:24 ` [PATCH 2/4] KVM: s390: Remove cond_resched() calls Heiko Carstens
@ 2026-07-27 16:57 ` sashiko-bot
0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-07-27 16:57 UTC (permalink / raw)
To: Heiko Carstens
Cc: Vasily Gorbik, linux-s390, Heiko Carstens, Alexander Gordeev
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule()
2026-07-27 16:24 ` [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule() Heiko Carstens
@ 2026-07-27 17:05 ` sashiko-bot
2026-07-28 7:05 ` Harald Freudenberger
1 sibling, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-07-27 17:05 UTC (permalink / raw)
To: Heiko Carstens
Cc: linux-s390, Vasily Gorbik, Heiko Carstens, Alexander Gordeev
> The comment for all cond_resched() calls in crypto code says that the
> process should be scheduled away to avoid instant re-invocation of a
> callback. This is not what cond_resched() would do or did.
>
> Replace cond_resched() with a call to schedule() which comes closer, but
> it looks more like this code wants an msleep() call.
>
> Note that with [1] cond_resched() is always compiled away and becomes a
> no-op.
>
> [1] commit 7dadeaa6e851 ("sched: Further restrict the preemption modes")
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260727162430.1599396-1-hca@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule()
2026-07-27 16:24 ` [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule() Heiko Carstens
2026-07-27 17:05 ` sashiko-bot
@ 2026-07-28 7:05 ` Harald Freudenberger
2026-07-28 8:29 ` Heiko Carstens
1 sibling, 1 reply; 16+ messages in thread
From: Harald Freudenberger @ 2026-07-28 7:05 UTC (permalink / raw)
To: Heiko Carstens
Cc: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Holger Dengler, Vineeth Vijayan,
Peter Oberparleiter, Janosch Frank, Claudio Imbrenda,
David Hildenbrand, Peter Zijlstra, linux-s390, linux-kernel
On 2026-07-27 18:24, Heiko Carstens wrote:
> The comment for all cond_resched() calls in crypto code says that the
> process should be scheduled away to avoid instant re-invocation of a
> callback. This is not what cond_resched() would do or did.
>
> Replace cond_resched() with a call to schedule() which comes closer,
> but
> it looks more like this code wants an msleep() call.
>
> Note that with [1] cond_resched() is always compiled away and becomes a
> no-op.
>
> [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..d02de945f864 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();
> + schedule();
> pr_debug("rescheduling request\n");
> return -ENOSPC;
> } else if (rc) {
> @@ -821,7 +821,7 @@ static int cbc_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();
> + schedule();
> pr_debug("rescheduling request\n");
> return -ENOSPC;
> } else if (rc) {
> @@ -1129,7 +1129,7 @@ static int ctr_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();
> + schedule();
> pr_debug("rescheduling request\n");
> return -ENOSPC;
> } else if (rc) {
> @@ -1572,7 +1572,7 @@ static int xts_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();
> + schedule();
> pr_debug("rescheduling request\n");
> return -ENOSPC;
> } else if (rc) {
> diff --git a/arch/s390/crypto/phmac_s390.c
> b/arch/s390/crypto/phmac_s390.c
> index 03ca33ffe6cc..bddfdcee6cb3 100644
> --- a/arch/s390/crypto/phmac_s390.c
> +++ b/arch/s390/crypto/phmac_s390.c
> @@ -895,7 +895,7 @@ static int phmac_do_one_request(struct
> crypto_engine *engine, void *areq)
> * tell scheduler to voluntarily give up the CPU here.
> */
> pr_debug("rescheduling request\n");
> - cond_resched();
> + schedule();
> return -ENOSPC;
> } else if (rc) {
> hwh_advance(hwh, rc);
> @@ -916,7 +916,7 @@ static int phmac_do_one_request(struct
> crypto_engine *engine, void *areq)
> * tell scheduler to voluntarily give up the CPU here.
> */
> pr_debug("rescheduling request\n");
> - cond_resched();
> + schedule();
> return -ENOSPC;
> }
> break;
Ok for me but we agreed to have all this code going via Herbert Xu.
So please address this patch to Herbert and the crypto mailing list.
And then I'll add my Reviewed-by.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule()
2026-07-28 7:05 ` Harald Freudenberger
@ 2026-07-28 8:29 ` Heiko Carstens
2026-07-28 8:53 ` Harald Freudenberger
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Heiko Carstens @ 2026-07-28 8:29 UTC (permalink / raw)
To: Harald Freudenberger
Cc: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Holger Dengler, Vineeth Vijayan,
Peter Oberparleiter, Janosch Frank, Claudio Imbrenda,
David Hildenbrand, Peter Zijlstra, linux-s390, linux-kernel
On Tue, Jul 28, 2026 at 09:05:16AM +0200, Harald Freudenberger wrote:
> On 2026-07-27 18:24, Heiko Carstens wrote:
> > The comment for all cond_resched() calls in crypto code says that the
> > process should be scheduled away to avoid instant re-invocation of a
> > callback. This is not what cond_resched() would do or did.
> >
> > Replace cond_resched() with a call to schedule() which comes closer, but
> > it looks more like this code wants an msleep() call.
> >
> > Note that with [1] cond_resched() is always compiled away and becomes a
> > no-op.
> >
> > [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(-)
...
> > - cond_resched();
> > + schedule();
> > return -ENOSPC;
> > }
> > break;
>
> Ok for me but we agreed to have all this code going via Herbert Xu.
> So please address this patch to Herbert and the crypto mailing list.
> And then I'll add my Reviewed-by.
Well, the main question still remains: why did the code use cond_resched()?
This doesn't make sense to me, at least when reading the comment. So to me
even the schedule() approach I proposed is questionable. Why didn't the code
use msleep() instead? Or can we just remove cond_resched() entirely without
any replacement?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule()
2026-07-28 8:29 ` Heiko Carstens
@ 2026-07-28 8:53 ` Harald Freudenberger
2026-07-28 9:51 ` Holger Dengler
2026-07-28 10:43 ` Peter Zijlstra
2 siblings, 0 replies; 16+ messages in thread
From: Harald Freudenberger @ 2026-07-28 8:53 UTC (permalink / raw)
To: Heiko Carstens
Cc: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Holger Dengler, Vineeth Vijayan,
Peter Oberparleiter, Janosch Frank, Claudio Imbrenda,
David Hildenbrand, Peter Zijlstra, linux-s390, linux-kernel
On 2026-07-28 10:29, Heiko Carstens wrote:
> On Tue, Jul 28, 2026 at 09:05:16AM +0200, Harald Freudenberger wrote:
>> On 2026-07-27 18:24, Heiko Carstens wrote:
>> > The comment for all cond_resched() calls in crypto code says that the
>> > process should be scheduled away to avoid instant re-invocation of a
>> > callback. This is not what cond_resched() would do or did.
>> >
>> > Replace cond_resched() with a call to schedule() which comes closer, but
>> > it looks more like this code wants an msleep() call.
>> >
>> > Note that with [1] cond_resched() is always compiled away and becomes a
>> > no-op.
>> >
>> > [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(-)
>
> ...
>
>> > - cond_resched();
>> > + schedule();
>> > return -ENOSPC;
>> > }
>> > break;
>>
>> Ok for me but we agreed to have all this code going via Herbert Xu.
>> So please address this patch to Herbert and the crypto mailing list.
>> And then I'll add my Reviewed-by.
>
> Well, the main question still remains: why did the code use
> cond_resched()?
>
> This doesn't make sense to me, at least when reading the comment. So to
> me
> even the schedule() approach I proposed is questionable. Why didn't the
> code
> use msleep() instead? Or can we just remove cond_resched() entirely
> without
> any replacement?
Reading through the "documentation" of cond_resched() versus schedule()
I would
suggest that schedule() is the better choice here. Maybe a mssleep()
would also
work here as we are running in our own crypto engine. However, I would
vote for
schedule() instead.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] s390/cio: Remove cond_resched() calls
2026-07-27 16:24 ` [PATCH 4/4] s390/cio: " Heiko Carstens
2026-07-27 16:54 ` sashiko-bot
@ 2026-07-28 9:28 ` Peter Oberparleiter
1 sibling, 0 replies; 16+ messages in thread
From: Peter Oberparleiter @ 2026-07-28 9:28 UTC (permalink / raw)
To: Heiko Carstens, Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Harald Freudenberger, Holger Dengler,
Vineeth Vijayan, Janosch Frank, Claudio Imbrenda,
David Hildenbrand, Peter Zijlstra
Cc: linux-s390, linux-kernel
On 27.07.2026 18:24, Heiko Carstens wrote:
> Since [1] cond_resched() is a no-op on s390. Remove all calls.
>
> [1] commit 7dadeaa6e851 ("sched: Further restrict the preemption modes")
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Makes sense, also given that this use of cond_resched() was added
specifically for the non-preempt case:
To quote b207f5a8f9a6 ("s390/cio: Relax subchannel scan loop"):
...
This processing can take a significant amount of time during which
no other task can run on the same CPU (unless CONFIG_PREEMPT has
been enabled).
...
Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
--
Peter Oberparleiter
Linux on IBM Z Development - IBM Germany R&D
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule()
2026-07-28 8:29 ` Heiko Carstens
2026-07-28 8:53 ` Harald Freudenberger
@ 2026-07-28 9:51 ` Holger Dengler
2026-07-28 12:06 ` Heiko Carstens
2026-07-28 10:43 ` Peter Zijlstra
2 siblings, 1 reply; 16+ messages in thread
From: Holger Dengler @ 2026-07-28 9:51 UTC (permalink / raw)
To: Heiko Carstens
Cc: Harald Freudenberger, Alexander Gordeev, Sven Schnelle,
Vasily Gorbik, Christian Borntraeger, Vineeth Vijayan,
Peter Oberparleiter, Janosch Frank, Claudio Imbrenda,
David Hildenbrand, Peter Zijlstra, linux-s390, linux-kernel
On 7/28/26 10:29, Heiko Carstens wrote:
> Well, the main question still remains: why did the code use cond_resched()?
>
> This doesn't make sense to me, at least when reading the comment. So to me
> even the schedule() approach I proposed is questionable. Why didn't the code
> use msleep() instead? Or can we just remove cond_resched() entirely without
> any replacement?
As the comment says, if possible, the scheduler should run any other
workload in case of a -EKEYEXPIRED, as the re-creation of the protected
key is in progress.
My current understanding was (and still is), that cond_resched() (or
even schedule()) is doing exactly that: give back the control to the
scheduler. If this is not the case, but msleep() is doing the trick, I
would vote for msleep().
Removing the code completely might also work. The situations, where the
protected keys expires are very rare (once per protected key after a
live guest relocation). These situations are hard to trigger and we
should test carefully, if the removal works in all cases.
--
Mit freundlichen Grüßen / Kind regards
Holger Dengler
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule()
2026-07-28 8:29 ` Heiko Carstens
2026-07-28 8:53 ` Harald Freudenberger
2026-07-28 9:51 ` Holger Dengler
@ 2026-07-28 10:43 ` Peter Zijlstra
2 siblings, 0 replies; 16+ messages in thread
From: Peter Zijlstra @ 2026-07-28 10:43 UTC (permalink / raw)
To: Heiko Carstens
Cc: Harald Freudenberger, Alexander Gordeev, Sven Schnelle,
Vasily Gorbik, Christian Borntraeger, Holger Dengler,
Vineeth Vijayan, Peter Oberparleiter, Janosch Frank,
Claudio Imbrenda, David Hildenbrand, linux-s390, linux-kernel
On Tue, Jul 28, 2026 at 10:29:48AM +0200, Heiko Carstens wrote:
> On Tue, Jul 28, 2026 at 09:05:16AM +0200, Harald Freudenberger wrote:
> > On 2026-07-27 18:24, Heiko Carstens wrote:
> > > The comment for all cond_resched() calls in crypto code says that the
> > > process should be scheduled away to avoid instant re-invocation of a
> > > callback. This is not what cond_resched() would do or did.
> > >
> > > Replace cond_resched() with a call to schedule() which comes closer, but
> > > it looks more like this code wants an msleep() call.
> > >
> > > Note that with [1] cond_resched() is always compiled away and becomes a
> > > no-op.
> > >
> > > [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(-)
>
> ...
>
> > > - cond_resched();
> > > + schedule();
> > > return -ENOSPC;
> > > }
> > > break;
> >
> > Ok for me but we agreed to have all this code going via Herbert Xu.
> > So please address this patch to Herbert and the crypto mailing list.
> > And then I'll add my Reviewed-by.
>
> Well, the main question still remains: why did the code use cond_resched()?
>
> This doesn't make sense to me, at least when reading the comment. So to me
> even the schedule() approach I proposed is questionable. Why didn't the code
> use msleep() instead? Or can we just remove cond_resched() entirely without
> any replacement?
Right, so suppose the task is a real-time FIFO-99 task (and there are no
deadline tasks), then the task will be the highest priority task in the
system and calling either cond_resched() or schedule() is a no-op, since
it will instantly re-select this same task for execution.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule()
2026-07-28 9:51 ` Holger Dengler
@ 2026-07-28 12:06 ` Heiko Carstens
0 siblings, 0 replies; 16+ messages in thread
From: Heiko Carstens @ 2026-07-28 12:06 UTC (permalink / raw)
To: Holger Dengler
Cc: Harald Freudenberger, Alexander Gordeev, Sven Schnelle,
Vasily Gorbik, Christian Borntraeger, Vineeth Vijayan,
Peter Oberparleiter, Janosch Frank, Claudio Imbrenda,
David Hildenbrand, Peter Zijlstra, linux-s390, linux-kernel
On Tue, Jul 28, 2026 at 11:51:04AM +0200, Holger Dengler wrote:
> On 7/28/26 10:29, Heiko Carstens wrote:
> > Well, the main question still remains: why did the code use cond_resched()?
> >
> > This doesn't make sense to me, at least when reading the comment. So to me
> > even the schedule() approach I proposed is questionable. Why didn't the code
> > use msleep() instead? Or can we just remove cond_resched() entirely without
> > any replacement?
>
> As the comment says, if possible, the scheduler should run any other
> workload in case of a -EKEYEXPIRED, as the re-creation of the protected key
> is in progress.
>
> My current understanding was (and still is), that cond_resched() (or even
> schedule()) is doing exactly that: give back the control to the scheduler.
This is not the case, and never was (maybe it was for schedule() before CFS,
but that's history). See also Peter's reply.
> If this is not the case, but msleep() is doing the trick, I would vote for
> msleep().
Ok, then I change cond_resched() to msleep(1).
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-07-28 12:06 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-27 16:24 [PATCH 0/4] s390: Remove or replace cond_resched() calls Heiko Carstens
2026-07-27 16:24 ` [PATCH 1/4] s390/crypto: Replace cond_resched() with schedule() Heiko Carstens
2026-07-27 17:05 ` sashiko-bot
2026-07-28 7:05 ` Harald Freudenberger
2026-07-28 8:29 ` Heiko Carstens
2026-07-28 8:53 ` Harald Freudenberger
2026-07-28 9:51 ` Holger Dengler
2026-07-28 12:06 ` Heiko Carstens
2026-07-28 10:43 ` Peter Zijlstra
2026-07-27 16:24 ` [PATCH 2/4] KVM: s390: Remove cond_resched() calls Heiko Carstens
2026-07-27 16:57 ` sashiko-bot
2026-07-27 16:24 ` [PATCH 3/4] " Heiko Carstens
2026-07-27 16:54 ` sashiko-bot
2026-07-27 16:24 ` [PATCH 4/4] s390/cio: " Heiko Carstens
2026-07-27 16:54 ` sashiko-bot
2026-07-28 9:28 ` Peter Oberparleiter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox