arch_vtime_task_switch() is a small function which is called
only from vtime_common_task_switch(), so it is worth inlining
Signed-off-by: Christophe Leroy <[email protected]>
---
v3: no change
v2: added a local pointer for get_accounting(prev) to avoid GCC to read it twice
arch/powerpc/include/asm/cputime.h | 16 +++++++++++++++-
arch/powerpc/kernel/time.c | 21 ---------------------
2 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
index 99b541865d8d..bc4903badb3f 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -47,9 +47,23 @@ static inline unsigned long cputime_to_usecs(const cputime_t ct)
* has to be populated in the new task
*/
#ifdef CONFIG_PPC64
+#define get_accounting(tsk) (&get_paca()->accounting)
static inline void arch_vtime_task_switch(struct task_struct *tsk) { }
#else
-void arch_vtime_task_switch(struct task_struct *tsk);
+#define get_accounting(tsk) (&task_thread_info(tsk)->accounting)
+/*
+ * Called from the context switch with interrupts disabled, to charge all
+ * accumulated times to the current process, and to prepare accounting on
+ * the next process.
+ */
+static inline void arch_vtime_task_switch(struct task_struct *prev)
+{
+ struct cpu_accounting_data *acct = get_accounting(current);
+ struct cpu_accounting_data *acct0 = get_accounting(prev);
+
+ acct->starttime = acct0->starttime;
+ acct->startspurr = acct0->startspurr;
+}
#endif
#endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 360e71d455cc..a3ed2eb99d88 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -163,12 +163,6 @@ EXPORT_SYMBOL(__cputime_usec_factor);
void (*dtl_consumer)(struct dtl_entry *, u64);
#endif
-#ifdef CONFIG_PPC64
-#define get_accounting(tsk) (&get_paca()->accounting)
-#else
-#define get_accounting(tsk) (&task_thread_info(tsk)->accounting)
-#endif
-
static void calc_cputime_factors(void)
{
struct div_result res;
@@ -421,21 +415,6 @@ void vtime_flush(struct task_struct *tsk)
acct->softirq_time = 0;
}
-#ifdef CONFIG_PPC32
-/*
- * Called from the context switch with interrupts disabled, to charge all
- * accumulated times to the current process, and to prepare accounting on
- * the next process.
- */
-void arch_vtime_task_switch(struct task_struct *prev)
-{
- struct cpu_accounting_data *acct = get_accounting(current);
-
- acct->starttime = get_accounting(prev)->starttime;
- acct->startspurr = get_accounting(prev)->startspurr;
-}
-#endif /* CONFIG_PPC32 */
-
#else /* ! CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
#define calc_cputime_factors()
#endif
--
2.13.3
scaled cputime is only meaningfull when the processor has
SPURR and/or PURR, which means only on PPC64.
Removing it on PPC32 significantly reduces the size of
vtime_account_system() and vtime_account_idle() on an 8xx:
Before:
00000114 l F .text 000000a8 vtime_delta
000004c0 g F .text 00000100 vtime_account_system
000005c0 g F .text 00000048 vtime_account_idle
After:
(vtime_delta gets inlined in the two functions)
00000418 g F .text 000000a0 vtime_account_system
000004b8 g F .text 00000054 vtime_account_idle
In terms of performance, we also get approximatly 5% improvement on task switch:
The following small benchmark app is run with perf stat:
void *thread(void *arg)
{
int i;
for (i = 0; i < atoi((char*)arg); i++)
pthread_yield();
}
int main(int argc, char **argv)
{
pthread_t th1, th2;
pthread_create(&th1, NULL, thread, argv[1]);
pthread_create(&th2, NULL, thread, argv[1]);
pthread_join(th1, NULL);
pthread_join(th2, NULL);
return 0;
}
Before the patch:
~# perf stat chrt -f 98 ./sched 100000
Performance counter stats for 'chrt -f 98 ./sched 100000':
8622.166272 task-clock (msec) # 0.955 CPUs utilized
200027 context-switches # 0.023 M/sec
After the patch:
~# perf stat chrt -f 98 ./sched 100000
Performance counter stats for 'chrt -f 98 ./sched 100000':
8207.090048 task-clock (msec) # 0.958 CPUs utilized
200025 context-switches # 0.024 M/sec
Signed-off-by: Christophe Leroy <[email protected]>
---
v3: Rebased following modifications in xmon.c
v2: added ifdefs in xmon to fix compilation error
arch/powerpc/Kconfig | 2 +-
arch/powerpc/include/asm/accounting.h | 4 ++++
arch/powerpc/include/asm/cputime.h | 2 ++
arch/powerpc/kernel/time.c | 29 +++++++++++++++++++++++------
arch/powerpc/xmon/xmon.c | 4 ++++
5 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 4a66d39addd4..87ebac147828 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -142,7 +142,7 @@ config PPC
select ARCH_HAS_PHYS_TO_DMA
select ARCH_HAS_PMEM_API if PPC64
select ARCH_HAS_MEMBARRIER_CALLBACKS
- select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE
+ select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC64
select ARCH_HAS_SG_CHAIN
select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION)
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
diff --git a/arch/powerpc/include/asm/accounting.h b/arch/powerpc/include/asm/accounting.h
index 3abcf98ed2e0..f1096d4cc658 100644
--- a/arch/powerpc/include/asm/accounting.h
+++ b/arch/powerpc/include/asm/accounting.h
@@ -15,8 +15,10 @@ struct cpu_accounting_data {
/* Accumulated cputime values to flush on ticks*/
unsigned long utime;
unsigned long stime;
+#ifdef ARCH_HAS_SCALED_CPUTIME
unsigned long utime_scaled;
unsigned long stime_scaled;
+#endif
unsigned long gtime;
unsigned long hardirq_time;
unsigned long softirq_time;
@@ -25,8 +27,10 @@ struct cpu_accounting_data {
/* Internal counters */
unsigned long starttime; /* TB value snapshot */
unsigned long starttime_user; /* TB value on exit to usermode */
+#ifdef ARCH_HAS_SCALED_CPUTIME
unsigned long startspurr; /* SPURR value snapshot */
unsigned long utime_sspurr; /* ->user_time when ->startspurr set */
+#endif
};
#endif
diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
index bc4903badb3f..8fd3c1338822 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -62,7 +62,9 @@ static inline void arch_vtime_task_switch(struct task_struct *prev)
struct cpu_accounting_data *acct0 = get_accounting(prev);
acct->starttime = acct0->starttime;
+#ifdef ARCH_HAS_SCALED_CPUTIME
acct->startspurr = acct0->startspurr;
+#endif
}
#endif
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index a3ed2eb99d88..7d6040233003 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -175,6 +175,7 @@ static void calc_cputime_factors(void)
* Read the SPURR on systems that have it, otherwise the PURR,
* or if that doesn't exist return the timebase value passed in.
*/
+#ifdef ARCH_HAS_SCALED_CPUTIME
static unsigned long read_spurr(unsigned long tb)
{
if (cpu_has_feature(CPU_FTR_SPURR))
@@ -183,6 +184,7 @@ static unsigned long read_spurr(unsigned long tb)
return mfspr(SPRN_PURR);
return tb;
}
+#endif
#ifdef CONFIG_PPC_SPLPAR
@@ -285,22 +287,28 @@ static unsigned long vtime_delta(struct task_struct *tsk,
unsigned long *stime_scaled,
unsigned long *steal_time)
{
- unsigned long now, nowscaled, deltascaled;
+ unsigned long now;
unsigned long stime;
+#ifdef ARCH_HAS_SCALED_CPUTIME
+ unsigned long nowscaled, deltascaled;
unsigned long utime, utime_scaled;
+#endif
struct cpu_accounting_data *acct = get_accounting(tsk);
WARN_ON_ONCE(!irqs_disabled());
now = mftb();
+#ifdef ARCH_HAS_SCALED_CPUTIME
nowscaled = read_spurr(now);
+#endif
stime = now - acct->starttime;
acct->starttime = now;
- deltascaled = nowscaled - acct->startspurr;
- acct->startspurr = nowscaled;
*steal_time = calculate_stolen_time(now);
+#ifdef ARCH_HAS_SCALED_CPUTIME
+ deltascaled = nowscaled - acct->startspurr;
+ acct->startspurr = nowscaled;
utime = acct->utime - acct->utime_sspurr;
acct->utime_sspurr = acct->utime;
@@ -325,6 +333,7 @@ static unsigned long vtime_delta(struct task_struct *tsk,
}
}
acct->utime_scaled += utime_scaled;
+#endif
return stime;
}
@@ -341,7 +350,9 @@ void vtime_account_system(struct task_struct *tsk)
if ((tsk->flags & PF_VCPU) && !irq_count()) {
acct->gtime += stime;
+#ifdef ARCH_HAS_SCALED_CPUTIME
acct->utime_scaled += stime_scaled;
+#endif
} else {
if (hardirq_count())
acct->hardirq_time += stime;
@@ -350,7 +361,9 @@ void vtime_account_system(struct task_struct *tsk)
else
acct->stime += stime;
+#ifdef ARCH_HAS_SCALED_CPUTIME
acct->stime_scaled += stime_scaled;
+#endif
}
}
EXPORT_SYMBOL_GPL(vtime_account_system);
@@ -378,8 +391,12 @@ void vtime_flush(struct task_struct *tsk)
if (acct->utime)
account_user_time(tsk, cputime_to_nsecs(acct->utime));
+#ifdef ARCH_HAS_SCALED_CPUTIME
if (acct->utime_scaled)
tsk->utimescaled += cputime_to_nsecs(acct->utime_scaled);
+ if (acct->stime_scaled)
+ tsk->stimescaled += cputime_to_nsecs(acct->stime_scaled);
+#endif
if (acct->gtime)
account_guest_time(tsk, cputime_to_nsecs(acct->gtime));
@@ -393,8 +410,6 @@ void vtime_flush(struct task_struct *tsk)
if (acct->stime)
account_system_index_time(tsk, cputime_to_nsecs(acct->stime),
CPUTIME_SYSTEM);
- if (acct->stime_scaled)
- tsk->stimescaled += cputime_to_nsecs(acct->stime_scaled);
if (acct->hardirq_time)
account_system_index_time(tsk, cputime_to_nsecs(acct->hardirq_time),
@@ -404,13 +419,15 @@ void vtime_flush(struct task_struct *tsk)
CPUTIME_SOFTIRQ);
acct->utime = 0;
+#ifdef ARCH_HAS_SCALED_CPUTIME
acct->utime_scaled = 0;
acct->utime_sspurr = 0;
+ acct->stime_scaled = 0;
+#endif
acct->gtime = 0;
acct->steal_time = 0;
acct->idle_time = 0;
acct->stime = 0;
- acct->stime_scaled = 0;
acct->hardirq_time = 0;
acct->softirq_time = 0;
}
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index c2e9270728c7..8ccfb2ae3aca 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2439,11 +2439,15 @@ static void dump_one_paca(int cpu)
DUMP(p, accounting.utime, "%#-*lx");
DUMP(p, accounting.stime, "%#-*lx");
+#ifdef ARCH_HAS_SCALED_CPUTIME
DUMP(p, accounting.utime_scaled, "%#-*lx");
+#endif
DUMP(p, accounting.starttime, "%#-*lx");
DUMP(p, accounting.starttime_user, "%#-*lx");
+#ifdef ARCH_HAS_SCALED_CPUTIME
DUMP(p, accounting.startspurr, "%#-*lx");
DUMP(p, accounting.utime_sspurr, "%#-*lx");
+#endif
DUMP(p, accounting.steal_time, "%#-*lx");
#undef DUMP
--
2.13.3
If CONFIG_PPC_SPLPAR is not selected, steal_time will always
be NUL, so accounting it is pointless
Signed-off-by: Christophe Leroy <[email protected]>
---
New in v3
arch/powerpc/kernel/time.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 7d6040233003..85d1b1ff45f3 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -346,7 +346,8 @@ void vtime_account_system(struct task_struct *tsk)
stime = vtime_delta(tsk, &stime_scaled, &steal_time);
stime -= min(stime, steal_time);
- acct->steal_time += steal_time;
+ if (IS_ENABLED(CONFIG_PPC_SPLPAR))
+ acct->steal_time += steal_time;
if ((tsk->flags & PF_VCPU) && !irq_count()) {
acct->gtime += stime;
@@ -401,7 +402,7 @@ void vtime_flush(struct task_struct *tsk)
if (acct->gtime)
account_guest_time(tsk, cputime_to_nsecs(acct->gtime));
- if (acct->steal_time)
+ if (IS_ENABLED(CONFIG_PPC_SPLPAR) && acct->steal_time)
account_steal_time(cputime_to_nsecs(acct->steal_time));
if (acct->idle_time)
@@ -425,7 +426,8 @@ void vtime_flush(struct task_struct *tsk)
acct->stime_scaled = 0;
#endif
acct->gtime = 0;
- acct->steal_time = 0;
+ if (IS_ENABLED(CONFIG_PPC_SPLPAR))
+ acct->steal_time = 0;
acct->idle_time = 0;
acct->stime = 0;
acct->hardirq_time = 0;
--
2.13.3
On Tue, 2018-05-29 at 16:19:14 UTC, Christophe Leroy wrote:
> arch_vtime_task_switch() is a small function which is called
> only from vtime_common_task_switch(), so it is worth inlining
>
> Signed-off-by: Christophe Leroy <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/60f1d2893ee6de65cdea609c84950b
cheers