[PATCH 0/9] s390/idle/vtime: Minor fixes and cleanups

[PATCH 0/9] s390/idle/vtime: Minor fixes and cleanups

[Heiko Carstens]

Frederic Weisbecker's cputime accounting patch series motivated me to
finally have a deeper look at the s390 cputime accounting code. The result
are two bug fixes, which are not at all critical, and a couple of cleanups
and improvements.

Heiko Carstens (9):
  s390/idle: Fix cpu idle exit cpu time accounting
  s390/vtime: Fix virtual timer forwarding
  s390/idle: Add comment for non obvious code
  s390/idle: Slightly optimize idle time accounting
  s390/idle: Inline update_timer_idle()
  s390/irq/idle: Remove psw bits early
  s390/vtime: Use __this_cpu_read() / get rid of READ_ONCE()
  s390/vtime: Use lockdep_assert_irqs_disabled() instead of BUG_ON()
  s390/idle: Remove psw_idle() prototype

 arch/s390/include/asm/idle.h  |  4 ++--
 arch/s390/include/asm/vtime.h | 34 ++++++++++++++++++++++++++++
 arch/s390/kernel/entry.h      |  2 --
 arch/s390/kernel/idle.c       | 25 +++++----------------
 arch/s390/kernel/irq.c        | 20 ++++++++++-------
 arch/s390/kernel/vtime.c      | 42 +++++++++--------------------------
 6 files changed, 63 insertions(+), 64 deletions(-)


base-commit: 9702969978695d9a699a1f34771580cdbb153b33
-- 
2.51.0

[PATCH 1/9] s390/idle: Fix cpu idle exit cpu time accounting

[Heiko Carstens]

With the conversion to generic entry [1] cpu idle exit cpu time accounting
was converted from assembly to C. This introduced an reversed order of cpu
time accounting.

On cpu idle exit the current accounting happens with the following call
chain:

-> do_io_irq()/do_ext_irq()
 -> irq_enter_rcu()
  -> account_hardirq_enter()
   -> vtime_account_irq()
    -> vtime_account_kernel()

vtime_account_kernel() accounts the passed cpu time since last_update_timer
as system time, and updates last_update_timer to the current cpu timer
value.

However the subsequent call of

 -> account_idle_time_irq()

will incorrectly subtract passed cpu time from timer_idle_enter to the
updated last_update_timer value from system_timer. Then last_update_timer
is updated to a sys_enter_timer, which means that last_update_timer goes
back in time.

Subsequently account_hardirq_exit() will account too much cpu time as
hardirq time. The sum of all accounted cpu times is still correct, however
some cpu time which was previously accounted as system time is now
accounted as hardirq time, plus there is the oddity that last_update_timer
goes back in time.

Restore previous behavior by extracting cpu time accounting code from
account_idle_time_irq() into a new update_timer_idle() function and call it
before irq_enter_rcu().

Fixes: 56e62a737028 ("s390: convert to generic entry") [1]
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/include/asm/idle.h |  1 +
 arch/s390/kernel/idle.c      | 13 +++++++++----
 arch/s390/kernel/irq.c       | 10 ++++++++--
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/s390/include/asm/idle.h b/arch/s390/include/asm/idle.h
index 09f763b9eb40..133059d9a949 100644
--- a/arch/s390/include/asm/idle.h
+++ b/arch/s390/include/asm/idle.h
@@ -23,5 +23,6 @@ extern struct device_attribute dev_attr_idle_count;
 extern struct device_attribute dev_attr_idle_time_us;
 
 void psw_idle(struct s390_idle_data *data, unsigned long psw_mask);
+void update_timer_idle(void);
 
 #endif /* _S390_IDLE_H */
diff --git a/arch/s390/kernel/idle.c b/arch/s390/kernel/idle.c
index 39cb8d0ae348..0f9e53f0a068 100644
--- a/arch/s390/kernel/idle.c
+++ b/arch/s390/kernel/idle.c
@@ -21,11 +21,10 @@
 
 static DEFINE_PER_CPU(struct s390_idle_data, s390_idle);
 
-void account_idle_time_irq(void)
+void update_timer_idle(void)
 {
 	struct s390_idle_data *idle = this_cpu_ptr(&s390_idle);
 	struct lowcore *lc = get_lowcore();
-	unsigned long idle_time;
 	u64 cycles_new[8];
 	int i;
 
@@ -35,13 +34,19 @@ void account_idle_time_irq(void)
 			this_cpu_add(mt_cycles[i], cycles_new[i] - idle->mt_cycles_enter[i]);
 	}
 
-	idle_time = lc->int_clock - idle->clock_idle_enter;
-
 	lc->steal_timer += idle->clock_idle_enter - lc->last_update_clock;
 	lc->last_update_clock = lc->int_clock;
 
 	lc->system_timer += lc->last_update_timer - idle->timer_idle_enter;
 	lc->last_update_timer = lc->sys_enter_timer;
+}
+
+void account_idle_time_irq(void)
+{
+	struct s390_idle_data *idle = this_cpu_ptr(&s390_idle);
+	unsigned long idle_time;
+
+	idle_time = get_lowcore()->int_clock - idle->clock_idle_enter;
 
 	/* Account time spent with enabled wait psw loaded as idle time. */
 	WRITE_ONCE(idle->idle_time, READ_ONCE(idle->idle_time) + idle_time);
diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
index bdf9c7cb5685..080e9285b337 100644
--- a/arch/s390/kernel/irq.c
+++ b/arch/s390/kernel/irq.c
@@ -146,6 +146,10 @@ void noinstr do_io_irq(struct pt_regs *regs)
 	struct pt_regs *old_regs = set_irq_regs(regs);
 	bool from_idle;
 
+	from_idle = test_and_clear_cpu_flag(CIF_ENABLED_WAIT);
+	if (from_idle)
+		update_timer_idle();
+
 	irq_enter_rcu();
 
 	if (user_mode(regs)) {
@@ -154,7 +158,6 @@ void noinstr do_io_irq(struct pt_regs *regs)
 			current->thread.last_break = regs->last_break;
 	}
 
-	from_idle = test_and_clear_cpu_flag(CIF_ENABLED_WAIT);
 	if (from_idle)
 		account_idle_time_irq();
 
@@ -182,6 +185,10 @@ void noinstr do_ext_irq(struct pt_regs *regs)
 	struct pt_regs *old_regs = set_irq_regs(regs);
 	bool from_idle;
 
+	from_idle = test_and_clear_cpu_flag(CIF_ENABLED_WAIT);
+	if (from_idle)
+		update_timer_idle();
+
 	irq_enter_rcu();
 
 	if (user_mode(regs)) {
@@ -194,7 +201,6 @@ void noinstr do_ext_irq(struct pt_regs *regs)
 	regs->int_parm = get_lowcore()->ext_params;
 	regs->int_parm_long = get_lowcore()->ext_params2;
 
-	from_idle = test_and_clear_cpu_flag(CIF_ENABLED_WAIT);
 	if (from_idle)
 		account_idle_time_irq();
 
-- 
2.51.0

[PATCH 2/9] s390/vtime: Fix virtual timer forwarding

[Heiko Carstens]

Since delayed accounting of system time [1] the virtual timer is
forwarded by do_account_vtime() but also vtime_account_kernel(),
vtime_account_softirq(), and vtime_account_hardirq(). This leads
to double accounting of system, guest, softirq, and hardirq time.

Remove accounting from the vtime_account*() family to restore old behavior.

There is only one user of the vtimer interface, which might explain
why nobody noticed this so far.

Fixes: b7394a5f4ce9 ("sched/cputime, s390: Implement delayed accounting of system time") [1]
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/vtime.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/s390/kernel/vtime.c b/arch/s390/kernel/vtime.c
index 234a0ba30510..122d30b10440 100644
--- a/arch/s390/kernel/vtime.c
+++ b/arch/s390/kernel/vtime.c
@@ -225,10 +225,6 @@ static u64 vtime_delta(void)
 	return timer - lc->last_update_timer;
 }
 
-/*
- * Update process times based on virtual cpu times stored by entry.S
- * to the lowcore fields user_timer, system_timer & steal_clock.
- */
 void vtime_account_kernel(struct task_struct *tsk)
 {
 	struct lowcore *lc = get_lowcore();
@@ -238,27 +234,17 @@ void vtime_account_kernel(struct task_struct *tsk)
 		lc->guest_timer += delta;
 	else
 		lc->system_timer += delta;
-
-	virt_timer_forward(delta);
 }
 EXPORT_SYMBOL_GPL(vtime_account_kernel);
 
 void vtime_account_softirq(struct task_struct *tsk)
 {
-	u64 delta = vtime_delta();
-
-	get_lowcore()->softirq_timer += delta;
-
-	virt_timer_forward(delta);
+	get_lowcore()->softirq_timer += vtime_delta();
 }
 
 void vtime_account_hardirq(struct task_struct *tsk)
 {
-	u64 delta = vtime_delta();
-
-	get_lowcore()->hardirq_timer += delta;
-
-	virt_timer_forward(delta);
+	get_lowcore()->hardirq_timer += vtime_delta();
 }
 
 /*
-- 
2.51.0

[PATCH 3/9] s390/idle: Add comment for non obvious code

[Heiko Carstens]

Add a comment to update_timer_idle() which describes why wall time (not
steal time) is added to steal_timer. This is not obvious and was reported
by Frederic Weisbecker.

Reported-by: Frederic Weisbecker <frederic@kernel.org>
Closes: https://lore.kernel.org/all/aXEVM-04lj0lntMr@localhost.localdomain/
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/idle.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/s390/kernel/idle.c b/arch/s390/kernel/idle.c
index 0f9e53f0a068..4e09f126d4fc 100644
--- a/arch/s390/kernel/idle.c
+++ b/arch/s390/kernel/idle.c
@@ -34,6 +34,15 @@ void update_timer_idle(void)
 			this_cpu_add(mt_cycles[i], cycles_new[i] - idle->mt_cycles_enter[i]);
 	}
 
+	/*
+	 * This is a bit subtle: Forward last_update_clock so it excludes idle
+	 * time. For correct steal time calculation in do_account_vtime() add
+	 * passed wall time before idle_enter to steal_timer:
+	 * During the passed wall time before idle_enter CPU time may have
+	 * been accounted to system, hardirq, softirq, etc. lowcore fields.
+	 * The accounted CPU times will be subtracted again from steal_timer
+	 * when accumulated steal time is calculated in do_account_vtime().
+	 */
 	lc->steal_timer += idle->clock_idle_enter - lc->last_update_clock;
 	lc->last_update_clock = lc->int_clock;
 
-- 
2.51.0

[PATCH 4/9] s390/idle: Slightly optimize idle time accounting

[Heiko Carstens]

Slightly optimize account_idle_time_irq() and update_timer_idle():

- Use fast single instruction __atomic64() primitives to update per
  cpu idle_time and idle_count, instead of READ_ONCE() / WRITE_ONCE()
  pairs

- stcctm() is an inline assembly with a full memory barrier. This
  leads to a not necessary extra dereference of smp_cpu_mtid in
  update_timer_idle(). Avoid this and read smp_cpu_mtid into a
  variable

- Use __this_cpu_add() instead of this_cpu_add() to avoid disabling /
  enabling of preemption several times in a loop in update_timer_idle().

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/idle.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/s390/kernel/idle.c b/arch/s390/kernel/idle.c
index 4e09f126d4fc..627d82dd900e 100644
--- a/arch/s390/kernel/idle.c
+++ b/arch/s390/kernel/idle.c
@@ -26,12 +26,13 @@ void update_timer_idle(void)
 	struct s390_idle_data *idle = this_cpu_ptr(&s390_idle);
 	struct lowcore *lc = get_lowcore();
 	u64 cycles_new[8];
-	int i;
+	int i, mtid;
 
-	if (smp_cpu_mtid) {
-		stcctm(MT_DIAG, smp_cpu_mtid, cycles_new);
-		for (i = 0; i < smp_cpu_mtid; i++)
-			this_cpu_add(mt_cycles[i], cycles_new[i] - idle->mt_cycles_enter[i]);
+	mtid = smp_cpu_mtid;
+	if (mtid) {
+		stcctm(MT_DIAG, mtid, cycles_new);
+		for (i = 0; i < mtid; i++)
+			__this_cpu_add(mt_cycles[i], cycles_new[i] - idle->mt_cycles_enter[i]);
 	}
 
 	/*
@@ -58,8 +59,8 @@ void account_idle_time_irq(void)
 	idle_time = get_lowcore()->int_clock - idle->clock_idle_enter;
 
 	/* Account time spent with enabled wait psw loaded as idle time. */
-	WRITE_ONCE(idle->idle_time, READ_ONCE(idle->idle_time) + idle_time);
-	WRITE_ONCE(idle->idle_count, READ_ONCE(idle->idle_count) + 1);
+	__atomic64_add(idle_time, &idle->idle_time);
+	__atomic64_add_const(1, &idle->idle_count);
 	account_idle_time(cputime_to_nsecs(idle_time));
 }
 
-- 
2.51.0

[PATCH 5/9] s390/idle: Inline update_timer_idle()

[Heiko Carstens]

Inline update_timer_idle() again to avoid an extra function call. This
way the generated code is close to old assembler version again.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/include/asm/idle.h  |  3 ++-
 arch/s390/include/asm/vtime.h | 34 ++++++++++++++++++++++++++++++++++
 arch/s390/kernel/entry.h      |  2 --
 arch/s390/kernel/idle.c       | 34 ++--------------------------------
 4 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/arch/s390/include/asm/idle.h b/arch/s390/include/asm/idle.h
index 133059d9a949..8c5aa7329461 100644
--- a/arch/s390/include/asm/idle.h
+++ b/arch/s390/include/asm/idle.h
@@ -19,10 +19,11 @@ struct s390_idle_data {
 	unsigned long mt_cycles_enter[8];
 };
 
+DECLARE_PER_CPU(struct s390_idle_data, s390_idle);
+
 extern struct device_attribute dev_attr_idle_count;
 extern struct device_attribute dev_attr_idle_time_us;
 
 void psw_idle(struct s390_idle_data *data, unsigned long psw_mask);
-void update_timer_idle(void);
 
 #endif /* _S390_IDLE_H */
diff --git a/arch/s390/include/asm/vtime.h b/arch/s390/include/asm/vtime.h
index 9d25fb35a042..b1db75d14e9d 100644
--- a/arch/s390/include/asm/vtime.h
+++ b/arch/s390/include/asm/vtime.h
@@ -2,6 +2,12 @@
 #ifndef _S390_VTIME_H
 #define _S390_VTIME_H
 
+#include <asm/lowcore.h>
+#include <asm/cpu_mf.h>
+#include <asm/idle.h>
+
+DECLARE_PER_CPU(u64, mt_cycles[8]);
+
 static inline void update_timer_sys(void)
 {
 	struct lowcore *lc = get_lowcore();
@@ -20,4 +26,32 @@ static inline void update_timer_mcck(void)
 	lc->last_update_timer = lc->mcck_enter_timer;
 }
 
+static inline void update_timer_idle(void)
+{
+	struct s390_idle_data *idle = this_cpu_ptr(&s390_idle);
+	struct lowcore *lc = get_lowcore();
+	u64 cycles_new[8];
+	int i, mtid;
+
+	mtid = smp_cpu_mtid;
+	if (mtid) {
+		stcctm(MT_DIAG, mtid, cycles_new);
+		for (i = 0; i < mtid; i++)
+			__this_cpu_add(mt_cycles[i], cycles_new[i] - idle->mt_cycles_enter[i]);
+	}
+	/*
+	 * This is a bit subtle: Forward last_update_clock so it excludes idle
+	 * time. For correct steal time calculation in do_account_vtime() add
+	 * passed wall time before idle_enter to steal_timer:
+	 * During the passed wall time before idle_enter CPU time may have
+	 * been accounted to system, hardirq, softirq, etc. lowcore fields.
+	 * The accounted CPU times will be subtracted again from steal_timer
+	 * when accumulated steal time is calculated in do_account_vtime().
+	 */
+	lc->steal_timer += idle->clock_idle_enter - lc->last_update_clock;
+	lc->last_update_clock = lc->int_clock;
+	lc->system_timer += lc->last_update_timer - idle->timer_idle_enter;
+	lc->last_update_timer = lc->sys_enter_timer;
+}
+
 #endif /* _S390_VTIME_H */
diff --git a/arch/s390/kernel/entry.h b/arch/s390/kernel/entry.h
index dd55cc6bbc28..fb67b4abe68c 100644
--- a/arch/s390/kernel/entry.h
+++ b/arch/s390/kernel/entry.h
@@ -56,8 +56,6 @@ long sys_s390_pci_mmio_write(unsigned long, const void __user *, size_t);
 long sys_s390_pci_mmio_read(unsigned long, void __user *, size_t);
 long sys_s390_sthyi(unsigned long function_code, void __user *buffer, u64 __user *return_code, unsigned long flags);
 
-DECLARE_PER_CPU(u64, mt_cycles[8]);
-
 unsigned long stack_alloc(void);
 void stack_free(unsigned long stack);
 
diff --git a/arch/s390/kernel/idle.c b/arch/s390/kernel/idle.c
index 627d82dd900e..1f1b06b6b4ef 100644
--- a/arch/s390/kernel/idle.c
+++ b/arch/s390/kernel/idle.c
@@ -15,41 +15,11 @@
 #include <trace/events/power.h>
 #include <asm/cpu_mf.h>
 #include <asm/cputime.h>
+#include <asm/idle.h>
 #include <asm/nmi.h>
 #include <asm/smp.h>
-#include "entry.h"
 
-static DEFINE_PER_CPU(struct s390_idle_data, s390_idle);
-
-void update_timer_idle(void)
-{
-	struct s390_idle_data *idle = this_cpu_ptr(&s390_idle);
-	struct lowcore *lc = get_lowcore();
-	u64 cycles_new[8];
-	int i, mtid;
-
-	mtid = smp_cpu_mtid;
-	if (mtid) {
-		stcctm(MT_DIAG, mtid, cycles_new);
-		for (i = 0; i < mtid; i++)
-			__this_cpu_add(mt_cycles[i], cycles_new[i] - idle->mt_cycles_enter[i]);
-	}
-
-	/*
-	 * This is a bit subtle: Forward last_update_clock so it excludes idle
-	 * time. For correct steal time calculation in do_account_vtime() add
-	 * passed wall time before idle_enter to steal_timer:
-	 * During the passed wall time before idle_enter CPU time may have
-	 * been accounted to system, hardirq, softirq, etc. lowcore fields.
-	 * The accounted CPU times will be subtracted again from steal_timer
-	 * when accumulated steal time is calculated in do_account_vtime().
-	 */
-	lc->steal_timer += idle->clock_idle_enter - lc->last_update_clock;
-	lc->last_update_clock = lc->int_clock;
-
-	lc->system_timer += lc->last_update_timer - idle->timer_idle_enter;
-	lc->last_update_timer = lc->sys_enter_timer;
-}
+DEFINE_PER_CPU(struct s390_idle_data, s390_idle);
 
 void account_idle_time_irq(void)
 {
-- 
2.51.0

[PATCH 6/9] s390/irq/idle: Remove psw bits early

[Heiko Carstens]

Remove wait, io, external interrupt bits early in do_io_irq()/do_ext_irq()
when previous context was idle. This saves one conditional branch and is
closer to the original old assembly code.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/irq.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
index 080e9285b337..084919a04e04 100644
--- a/arch/s390/kernel/irq.c
+++ b/arch/s390/kernel/irq.c
@@ -147,8 +147,10 @@ void noinstr do_io_irq(struct pt_regs *regs)
 	bool from_idle;
 
 	from_idle = test_and_clear_cpu_flag(CIF_ENABLED_WAIT);
-	if (from_idle)
+	if (from_idle) {
 		update_timer_idle();
+		regs->psw.mask &= ~(PSW_MASK_EXT | PSW_MASK_IO | PSW_MASK_WAIT);
+	}
 
 	irq_enter_rcu();
 
@@ -174,9 +176,6 @@ void noinstr do_io_irq(struct pt_regs *regs)
 
 	set_irq_regs(old_regs);
 	irqentry_exit(regs, state);
-
-	if (from_idle)
-		regs->psw.mask &= ~(PSW_MASK_EXT | PSW_MASK_IO | PSW_MASK_WAIT);
 }
 
 void noinstr do_ext_irq(struct pt_regs *regs)
@@ -186,8 +185,10 @@ void noinstr do_ext_irq(struct pt_regs *regs)
 	bool from_idle;
 
 	from_idle = test_and_clear_cpu_flag(CIF_ENABLED_WAIT);
-	if (from_idle)
+	if (from_idle) {
 		update_timer_idle();
+		regs->psw.mask &= ~(PSW_MASK_EXT | PSW_MASK_IO | PSW_MASK_WAIT);
+	}
 
 	irq_enter_rcu();
 
@@ -209,9 +210,6 @@ void noinstr do_ext_irq(struct pt_regs *regs)
 	irq_exit_rcu();
 	set_irq_regs(old_regs);
 	irqentry_exit(regs, state);
-
-	if (from_idle)
-		regs->psw.mask &= ~(PSW_MASK_EXT | PSW_MASK_IO | PSW_MASK_WAIT);
 }
 
 static void show_msi_interrupt(struct seq_file *p, int irq)
-- 
2.51.0

[PATCH 7/9] s390/vtime: Use __this_cpu_read() / get rid of READ_ONCE()

[Heiko Carstens]

do_account_vtime() runs always with interrupts disabled, therefore use
__this_cpu_read() instead of this_cpu_read() to get rid of a pointless
preempt_disable() / preempt_enable() pair.

Also there are no concurrent writers to the cpu time accounting fields
in lowcore. Therefore get rid of READ_ONCE() usages.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/vtime.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/s390/kernel/vtime.c b/arch/s390/kernel/vtime.c
index 122d30b10440..4111ff4d727c 100644
--- a/arch/s390/kernel/vtime.c
+++ b/arch/s390/kernel/vtime.c
@@ -137,23 +137,16 @@ static int do_account_vtime(struct task_struct *tsk)
 		lc->system_timer += timer;
 
 	/* Update MT utilization calculation */
-	if (smp_cpu_mtid &&
-	    time_after64(jiffies_64, this_cpu_read(mt_scaling_jiffies)))
+	if (smp_cpu_mtid && time_after64(jiffies_64, __this_cpu_read(mt_scaling_jiffies)))
 		update_mt_scaling();
 
 	/* Calculate cputime delta */
-	user = update_tsk_timer(&tsk->thread.user_timer,
-				READ_ONCE(lc->user_timer));
-	guest = update_tsk_timer(&tsk->thread.guest_timer,
-				 READ_ONCE(lc->guest_timer));
-	system = update_tsk_timer(&tsk->thread.system_timer,
-				  READ_ONCE(lc->system_timer));
-	hardirq = update_tsk_timer(&tsk->thread.hardirq_timer,
-				   READ_ONCE(lc->hardirq_timer));
-	softirq = update_tsk_timer(&tsk->thread.softirq_timer,
-				   READ_ONCE(lc->softirq_timer));
-	lc->steal_timer +=
-		clock - user - guest - system - hardirq - softirq;
+	user = update_tsk_timer(&tsk->thread.user_timer, lc->user_timer);
+	guest = update_tsk_timer(&tsk->thread.guest_timer, lc->guest_timer);
+	system = update_tsk_timer(&tsk->thread.system_timer, lc->system_timer);
+	hardirq = update_tsk_timer(&tsk->thread.hardirq_timer, lc->hardirq_timer);
+	softirq = update_tsk_timer(&tsk->thread.softirq_timer, lc->softirq_timer);
+	lc->steal_timer += clock - user - guest - system - hardirq - softirq;
 
 	/* Push account value */
 	if (user) {
-- 
2.51.0

[PATCH 8/9] s390/vtime: Use lockdep_assert_irqs_disabled() instead of BUG_ON()

[Heiko Carstens]

Use lockdep_assert_irqs_disabled() instead of BUG_ON(). This avoids
crashing the kernel, and generates better code if CONFIG_PROVE_LOCKING
is disabled.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/vtime.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/s390/kernel/vtime.c b/arch/s390/kernel/vtime.c
index 4111ff4d727c..bf48744d0912 100644
--- a/arch/s390/kernel/vtime.c
+++ b/arch/s390/kernel/vtime.c
@@ -48,8 +48,7 @@ static inline void set_vtimer(u64 expires)
 
 static inline int virt_timer_forward(u64 elapsed)
 {
-	BUG_ON(!irqs_disabled());
-
+	lockdep_assert_irqs_disabled();
 	if (list_empty(&virt_timer_list))
 		return 0;
 	elapsed = atomic64_add_return(elapsed, &virt_timer_elapsed);
-- 
2.51.0

[PATCH 9/9] s390/idle: Remove psw_idle() prototype

[Heiko Carstens]

psw_idle() does not exist anymore. Remove its prototype.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/include/asm/idle.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/s390/include/asm/idle.h b/arch/s390/include/asm/idle.h
index 8c5aa7329461..32536ee34aa0 100644
--- a/arch/s390/include/asm/idle.h
+++ b/arch/s390/include/asm/idle.h
@@ -24,6 +24,4 @@ DECLARE_PER_CPU(struct s390_idle_data, s390_idle);
 extern struct device_attribute dev_attr_idle_count;
 extern struct device_attribute dev_attr_idle_time_us;
 
-void psw_idle(struct s390_idle_data *data, unsigned long psw_mask);
-
 #endif /* _S390_IDLE_H */
-- 
2.51.0

Re: [PATCH 0/9] s390/idle/vtime: Minor fixes and cleanups

[Sven Schnelle]

Heiko Carstens <hca@linux.ibm.com> writes:

> Frederic Weisbecker's cputime accounting patch series motivated me to
> finally have a deeper look at the s390 cputime accounting code. The result
> are two bug fixes, which are not at all critical, and a couple of cleanups
> and improvements.
>
> Heiko Carstens (9):
>   s390/idle: Fix cpu idle exit cpu time accounting
>   s390/vtime: Fix virtual timer forwarding
>   s390/idle: Add comment for non obvious code
>   s390/idle: Slightly optimize idle time accounting
>   s390/idle: Inline update_timer_idle()
>   s390/irq/idle: Remove psw bits early
>   s390/vtime: Use __this_cpu_read() / get rid of READ_ONCE()
>   s390/vtime: Use lockdep_assert_irqs_disabled() instead of BUG_ON()
>   s390/idle: Remove psw_idle() prototype
>
>  arch/s390/include/asm/idle.h  |  4 ++--
>  arch/s390/include/asm/vtime.h | 34 ++++++++++++++++++++++++++++
>  arch/s390/kernel/entry.h      |  2 --
>  arch/s390/kernel/idle.c       | 25 +++++----------------
>  arch/s390/kernel/irq.c        | 20 ++++++++++-------
>  arch/s390/kernel/vtime.c      | 42 +++++++++--------------------------
>  6 files changed, 63 insertions(+), 64 deletions(-)
>
>
> base-commit: 9702969978695d9a699a1f34771580cdbb153b33

For the series:

Reviewed-by: Sven Schnelle <svens@linux.ibm.com>

Re: [PATCH 1/9] s390/idle: Fix cpu idle exit cpu time accounting

[Frederic Weisbecker]

Le Wed, Feb 18, 2026 at 03:20:04PM +0100, Heiko Carstens a écrit :
> With the conversion to generic entry [1] cpu idle exit cpu time accounting
> was converted from assembly to C. This introduced an reversed order of cpu
> time accounting.
> 
> On cpu idle exit the current accounting happens with the following call
> chain:
> 
> -> do_io_irq()/do_ext_irq()
>  -> irq_enter_rcu()
>   -> account_hardirq_enter()
>    -> vtime_account_irq()
>     -> vtime_account_kernel()
> 
> vtime_account_kernel() accounts the passed cpu time since last_update_timer
> as system time, and updates last_update_timer to the current cpu timer
> value.
> 
> However the subsequent call of
> 
>  -> account_idle_time_irq()
> 
> will incorrectly subtract passed cpu time from timer_idle_enter to the
> updated last_update_timer value from system_timer. Then last_update_timer
> is updated to a sys_enter_timer, which means that last_update_timer goes
> back in time.
> 
> Subsequently account_hardirq_exit() will account too much cpu time as
> hardirq time. The sum of all accounted cpu times is still correct, however
> some cpu time which was previously accounted as system time is now
> accounted as hardirq time, plus there is the oddity that last_update_timer
> goes back in time.
> 
> Restore previous behavior by extracting cpu time accounting code from
> account_idle_time_irq() into a new update_timer_idle() function and call it
> before irq_enter_rcu().
> 
> Fixes: 56e62a737028 ("s390: convert to generic entry") [1]
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

-- 
Frederic Weisbecker
SUSE Labs

Re: [PATCH 2/9] s390/vtime: Fix virtual timer forwarding

[Frederic Weisbecker]

Le Wed, Feb 18, 2026 at 03:20:05PM +0100, Heiko Carstens a écrit :
> Since delayed accounting of system time [1] the virtual timer is
> forwarded by do_account_vtime() but also vtime_account_kernel(),
> vtime_account_softirq(), and vtime_account_hardirq(). This leads
> to double accounting of system, guest, softirq, and hardirq time.
> 
> Remove accounting from the vtime_account*() family to restore old behavior.
> 
> There is only one user of the vtimer interface, which might explain
> why nobody noticed this so far.
> 
> Fixes: b7394a5f4ce9 ("sched/cputime, s390: Implement delayed accounting of system time") [1]
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

-- 
Frederic Weisbecker
SUSE Labs

Re: [PATCH 3/9] s390/idle: Add comment for non obvious code

[Frederic Weisbecker]

Le Wed, Feb 18, 2026 at 03:20:06PM +0100, Heiko Carstens a écrit :
> Add a comment to update_timer_idle() which describes why wall time (not
> steal time) is added to steal_timer. This is not obvious and was reported
> by Frederic Weisbecker.
> 
> Reported-by: Frederic Weisbecker <frederic@kernel.org>
> Closes: https://lore.kernel.org/all/aXEVM-04lj0lntMr@localhost.localdomain/
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

-- 
Frederic Weisbecker
SUSE Labs

Re: [PATCH 7/9] s390/vtime: Use __this_cpu_read() / get rid of READ_ONCE()

[Frederic Weisbecker]

Le Wed, Feb 18, 2026 at 03:20:10PM +0100, Heiko Carstens a écrit :
> do_account_vtime() runs always with interrupts disabled, therefore use
> __this_cpu_read() instead of this_cpu_read() to get rid of a pointless
> preempt_disable() / preempt_enable() pair.
> 
> Also there are no concurrent writers to the cpu time accounting fields
> in lowcore. Therefore get rid of READ_ONCE() usages.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

-- 
Frederic Weisbecker
SUSE Labs

Re: [PATCH 5/9] s390/idle: Inline update_timer_idle()

[Frederic Weisbecker]

Le Wed, Feb 18, 2026 at 03:20:08PM +0100, Heiko Carstens a écrit :
> Inline update_timer_idle() again to avoid an extra function call. This
> way the generated code is close to old assembler version again.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

-- 
Frederic Weisbecker
SUSE Labs

Re: [PATCH 8/9] s390/vtime: Use lockdep_assert_irqs_disabled() instead of BUG_ON()

[Frederic Weisbecker]

Le Wed, Feb 18, 2026 at 03:20:11PM +0100, Heiko Carstens a écrit :
> Use lockdep_assert_irqs_disabled() instead of BUG_ON(). This avoids
> crashing the kernel, and generates better code if CONFIG_PROVE_LOCKING
> is disabled.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

-- 
Frederic Weisbecker
SUSE Labs

Re: [PATCH 9/9] s390/idle: Remove psw_idle() prototype

[Frederic Weisbecker]

Le Wed, Feb 18, 2026 at 03:20:12PM +0100, Heiko Carstens a écrit :
> psw_idle() does not exist anymore. Remove its prototype.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

-- 
Frederic Weisbecker
SUSE Labs