sched,time: reduce nohz_full syscall overhead 40%
Subsystems (2)
v3 cover sched,time: reduce nohz_full syscall overhead 40%
(v3: address comments raised by Frederic)
Running with nohz_full introduces a fair amount of overhead. Specifically, various things that are usually done from the timer interrupt are now done at syscall, irq, and guest entry and exit times.
However, some of the code that is called every single time has only ever worked at jiffy resolution. The code in __acct_update_integrals was also doing some unnecessary calculations.
Getting rid of the unnecessary calculations, without changing any of the functionality in __acct_update_integrals gets us about an 11% win.
Not calling the time statistics updating code more than once per jiffy, like is done on housekeeping CPUs and on all the CPUs of a non-nohz_full system, shaves off a further 30%.
I tested this series with a microbenchmark calling an invalid syscall number ten million times in a row, on a nohz_full cpu.
Run times for the microbenchmark:
4.4 3.8 seconds
4.5-rc1 3.7 seconds
4.5-rc1 + first patch 3.3 seconds
4.5-rc1 + first 3 patches 3.1 seconds
4.5-rc1 + all patches 2.3 seconds
PATCH v3 1/4 sched,time: remove non-power-of-two divides from __acct_update_integrals
From: Rik van Riel <[email protected]>
When running a microbenchmark calling an invalid syscall number in a loop, on a nohz_full CPU, we spend a full 9% of our CPU time in __acct_update_integrals.
This function converts cputime_t to jiffies, to a timeval, only to convert the timeval back to microseconds before discarding it.
This patch leaves __acct_update_integrals functionally equivalent, but speeds things up by about 12%, with 10 million calls to an invalid syscall number dropping from 3.7 to 3.25 seconds.
--- kernel/tsacct.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)
kernel/tsacct.c +14 -10
@@ -93,9 +93,9 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *p) {struct mm_struct *mm; -/* convert pages-usec to Mbyte-usec */ - stats->coremem = p->acct_rss_mem1 *PAGE_SIZE /MB ; - stats->virtmem = p->acct_vm_mem1 *PAGE_SIZE /MB ; +/* convert pages-nsec/1024 to Mbyte-usec, see __acct_update_integrals */ + stats->coremem = p->acct_rss_mem1 *PAGE_SIZE / (1000 *KB ); + stats->virtmem = p->acct_vm_mem1 *PAGE_SIZE / (1000 *KB ); mm =get_task_mm (p);if (mm) {/* adjust to KB unit */
@@ -125,22 +125,26 @@ static void __acct_update_integrals(struct task_struct *tsk, {if (likely (tsk->mm)) {cputime_t time, dtime; -struct timeval value;unsigned long flags;u64 delta;local_irq_save (flags); time = stime + utime; dtime = time - tsk->acct_timexpd; -jiffies_to_timeval (cputime_to_jiffies (dtime), &value); - delta = value.tv_sec; - delta = delta *USEC_PER_SEC + value.tv_usec; +/* Avoid division: cputime_t is often in nanoseconds already. */ + delta =cputime_to_nsecs (dtime); -if (delta ==0 ) +if (delta <TICK_NSEC ) goto out; + tsk->acct_timexpd = time; - tsk->acct_rss_mem1 += delta *get_mm_rss (tsk->mm); - tsk->acct_vm_mem1 += delta * tsk->mm->total_vm; +/* +* Divide by 1024 to avoid overflow, and to avoid division. +* The final unit reported to userspace is Mbyte-usecs, +* the rest of the math is done in xacct_add_tsk. +*/ + tsk->acct_rss_mem1 += delta *get_mm_rss (tsk->mm) >>10 ; + tsk->acct_vm_mem1 += delta * tsk->mm->total_vm >>10 ; out:local_irq_restore (flags); }
Signature · 1 line
2.5.0
kbuild test robot
build botRe: [PATCH 1/4] sched,time: remove non-power-of-two divides from __acct_update_integrals
reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree…
Hi Rik,
[auto build test ERROR on tip/sched/core]
[also build test ERROR on v4.5-rc2 next-20160129]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/riel-redhat-com/sched-time-reduce-nohz_full-syscall-overhead-40/20160201-101609 config: sh-sh7757lcr_defconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh
All errors (new ones prefixed by >>):
kernel/built-in.o: In function `get_mm_hiwater_vm':
>> include/linux/mm.h:1377: undefined reference to `__udivdi3'
vim +1377 include/linux/mm.h
172703b0 Matt Fleming 2011-05-24 1361 atomic_long_dec(&mm->rss_stat.count[member]);
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1362 }
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1363
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1364 static inline unsigned long get_mm_rss(struct mm_struct *mm)
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1365 {
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1366 return get_mm_counter(mm, MM_FILEPAGES) +
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1367 get_mm_counter(mm, MM_ANONPAGES);
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1368 }
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1369
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1370 static inline unsigned long get_mm_hiwater_rss(struct mm_struct *mm)
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1371 {
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1372 return max(mm->hiwater_rss, get_mm_rss(mm));
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1373 }
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1374
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1375 static inline unsigned long get_mm_hiwater_vm(struct mm_struct *mm)
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1376 {
d559db08 KAMEZAWA Hiroyuki 2010-03-05 @1377 return max(mm->hiwater_vm, mm->total_vm);
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1378 }
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1379
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1380 static inline void update_hiwater_rss(struct mm_struct *mm)
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1381 {
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1382 unsigned long _rss = get_mm_rss(mm);
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1383
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1384 if ((mm)->hiwater_rss < _rss)
d559db08 KAMEZAWA Hiroyuki 2010-03-05 1385 (mm)->hiwater_rss = _rss;:::::: The code at line 1377 was first introduced by commit :::::: d559db086ff5be9bcc259e5aa50bf3d881eaf1d1 mm: clean up mm_counter
:::::: TO: KAMEZAWA Hiroyuki <[email protected]> :::::: CC: Linus Torvalds <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH 1/4] sched,time: remove non-power-of-two divides from __acct_update_integrals
In reply to [email protected]kernel/tsacct.c void xacct_add_tsk(struct taskstats *stats, struct task_struct *p) · line 98
Quoted 10 lines · On Sun, 31 Jan 2016, [email protected] wrote:
kernel/tsacct.c +3
-3@@ -93,9 +93,9 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *p) {struct mm_struct *mm; -/* convert pages-usec to Mbyte-usec */ - stats->coremem = p->acct_rss_mem1 *PAGE_SIZE /MB ; - stats->virtmem = p->acct_vm_mem1 *PAGE_SIZE /MB ; +/* convert pages-nsec/1024 to Mbyte-usec, see __acct_update_integrals */ + stats->coremem = p->acct_rss_mem1 *PAGE_SIZE / (1000 *KB ); + stats->virtmem = p->acct_vm_mem1 *PAGE_SIZE / (1000 *KB );
You replace "/ (1024 * 1024)" by "/ (1000 * 1024). So that's introducing a non power of 2 division instead of removing one and wont compile on systems which do not have a 64/32 division in hardware.
Thanks,
tglx
Peter Zijlstra
maintainerRe: [PATCH 1/4] sched,time: remove non-power-of-two divides from __acct_update_integrals
Quoted 5 lines · Thomas Gleixner
Quoted 4 lines · On Sun, 31 Jan 2016, [email protected] wrote:
kernel/tsacct.c +0
-0@@ -93,9 +93,9 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *p) {struct mm_struct *mm;
Quoted 10 lines
kernel/tsacct.c void xacct_add_tsk(struct taskstats *stats, struct task_struct *p) · line 98
Quoted 6 lines
kernel/tsacct.c +3
-3-/* convert pages-usec to Mbyte-usec */ - stats->coremem = p->acct_rss_mem1 *PAGE_SIZE /MB ; - stats->virtmem = p->acct_vm_mem1 *PAGE_SIZE /MB ; +/* convert pages-nsec/1024 to Mbyte-usec, see __acct_update_integrals */ + stats->coremem = p->acct_rss_mem1 *PAGE_SIZE / (1000 *KB ); + stats->virtmem = p->acct_vm_mem1 *PAGE_SIZE / (1000 *KB );You replace "/ (1024 * 1024)" by "/ (1000 * 1024). So that's introducing a non power of 2 division instead of removing one and wont compile on systems which do not have a 64/32 division in hardware.
Yep, so that needs to be fixed to use do_div(). But the reason for this is that this is the consumer side of these stats and therefore rarely executed.
This patch effectively moves a div out of the fast path into the slow path.
Re: [PATCH 1/4] sched,time: remove non-power-of-two divides from __acct_update_integrals
Quoted 17 lines · Peter Zijlstra
Quoted 5 lines · Thomas Gleixner
Quoted 4 lines · On Sun, 31 Jan 2016, [email protected] wrote:
kernel/tsacct.c +0
-0@@ -93,9 +93,9 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *p) {struct mm_struct *mm;Quoted 10 lines
kernel/tsacct.c void xacct_add_tsk(struct taskstats *stats, struct task_struct *p) · line 98
Quoted 6 lines
kernel/tsacct.c +3
-3-/* convert pages-usec to Mbyte-usec */ - stats->coremem = p->acct_rss_mem1 *PAGE_SIZE /MB ; - stats->virtmem = p->acct_vm_mem1 *PAGE_SIZE /MB ; +/* convert pages-nsec/1024 to Mbyte-usec, see __acct_update_integrals */ + stats->coremem = p->acct_rss_mem1 *PAGE_SIZE / (1000 *KB ); + stats->virtmem = p->acct_vm_mem1 *PAGE_SIZE / (1000 *KB );You replace "/ (1024 * 1024)" by "/ (1000 * 1024). So that's introducing a non power of 2 division instead of removing one and wont compile on systems which do not have a 64/32 division in hardware.
Quoted 6 lines
Yep, so that needs to be fixed to use
do_div(). But the reason for this is that this is the consumer side of these stats and therefore rarely executed.This patch effectively moves a div out of the fast path into the slow path.
Yeah, noticed after hitting Send :)
Re: [PATCH 1/4] sched,time: remove non-power-of-two divides from __acct_update_integrals
In reply to Peter ZijlstraQuoted 17 lines · Peter Zijlstra
Quoted 5 lines · Thomas Gleixner
Quoted 4 lines · On Sun, 31 Jan 2016, [email protected] wrote:
kernel/tsacct.c +0
-0@@ -93,9 +93,9 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *p) {struct mm_struct *mm;Quoted 10 lines
kernel/tsacct.c void xacct_add_tsk(struct taskstats *stats, struct task_struct *p) · line 98
Quoted 6 lines
kernel/tsacct.c +3
-3-/* convert pages-usec to Mbyte-usec */ - stats->coremem = p->acct_rss_mem1 *PAGE_SIZE /MB ; - stats->virtmem = p->acct_vm_mem1 *PAGE_SIZE /MB ; +/* convert pages-nsec/1024 to Mbyte-usec, see __acct_update_integrals */ + stats->coremem = p->acct_rss_mem1 *PAGE_SIZE / (1000 *KB ); + stats->virtmem = p->acct_vm_mem1 *PAGE_SIZE / (1000 *KB );You replace "/ (1024 * 1024)" by "/ (1000 * 1024). So that's introducing a non power of 2 division instead of removing one and wont compile on systems which do not have a 64/32 division in hardware.
Quoted 3 lines
Yep, so that needs to be fixed to use
do_div(). But the reason for this is that this is the consumer side of these stats and therefore rarely executed.
Before I send in a v4 with do_div in that location, are there any other changes you would like me to make to the code?
Signature · 1 line
All rights reversed
Peter Zijlstra
maintainerRe: [PATCH 1/4] sched,time: remove non-power-of-two divides from __acct_update_integrals
Quoted 2 lines · Rik van Riel
Before I send in a v4 with
do_divin that location, are there any other changes you would like me to make to the code?
The WARN_ON(!irqs_disabled()) run and adding irq_save/restore to acct_update_integrals() would be good :-)
PATCH v3 2/4 acct,time: change indentation in __acct_update_integrals
In reply to [email protected]From: Rik van Riel <[email protected]>
Change the indentation in __acct_update_integrals to make the function a little easier to read.
--- kernel/tsacct.c | 51 ++++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 25 deletions(-)
kernel/tsacct.c +26 -25
@@ -123,31 +123,32 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *p)static void __acct_update_integrals (struct task_struct *tsk,cputime_t utime,cputime_t stime) { -if (likely (tsk->mm)) { -cputime_t time, dtime; -unsigned long flags; -u64 delta; - -local_irq_save (flags); - time = stime + utime; - dtime = time - tsk->acct_timexpd; -/* Avoid division: cputime_t is often in nanoseconds already. */ - delta =cputime_to_nsecs (dtime); - -if (delta <TICK_NSEC ) - goto out; - - tsk->acct_timexpd = time; -/* -* Divide by 1024 to avoid overflow, and to avoid division. -* The final unit reported to userspace is Mbyte-usecs, -* the rest of the math is done in xacct_add_tsk. -*/ - tsk->acct_rss_mem1 += delta *get_mm_rss (tsk->mm) >>10 ; - tsk->acct_vm_mem1 += delta * tsk->mm->total_vm >>10 ; - out: -local_irq_restore (flags); - } +cputime_t time, dtime; +unsigned long flags; +u64 delta; + +if (!likely (tsk->mm)) +return ; + +local_irq_save (flags); + time = stime + utime; + dtime = time - tsk->acct_timexpd; +/* Avoid division: cputime_t is often in nanoseconds already. */ + delta =cputime_to_nsecs (dtime); + +if (delta <TICK_NSEC ) + goto out; + + tsk->acct_timexpd = time; +/* +* Divide by 1024 to avoid overflow, and to avoid division. +* The final unit reported to userspace is Mbyte-usecs, +* the rest of the math is done in xacct_add_tsk. +*/ + tsk->acct_rss_mem1 += delta *get_mm_rss (tsk->mm) >>10 ; + tsk->acct_vm_mem1 += delta * tsk->mm->total_vm >>10 ; +out: +local_irq_restore (flags); }/**
Signature · 1 line
2.5.0
PATCH v3 3/4 time,acct: drop irq save & restore from __acct_update_integrals
In reply to [email protected]From: Rik van Riel <[email protected]>
It looks like all the call paths that lead to __acct_update_integrals already have irqs disabled, and __acct_update_integrals does not need to disable irqs itself.
This is very convenient since about half the CPU time left in this function was spent in local_irq_save alone.
Performance of a microbenchmark that calls an invalid syscall ten million times in a row on a nohz_full CPU improves 21% vs. 4.5-rc1 with both the removal of divisions from __acct_update_integrals and this patch, with runtime dropping from 3.7 to 2.9 seconds.
With these patches applied, the highest remaining cpu user in the trace is native_sched_clock, which is addressed in the next patch.
--- kernel/tsacct.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
kernel/tsacct.c +1 -5
@@ -124,20 +124,18 @@ static void __acct_update_integrals(struct task_struct *tsk, cputime_t utime, cputime_t stime) {cputime_t time, dtime; -unsigned long flags;u64 delta;if (!likely (tsk->mm))return ; -local_irq_save (flags); time = stime + utime; dtime = time - tsk->acct_timexpd;/* Avoid division: cputime_t is often in nanoseconds already. */ delta =cputime_to_nsecs (dtime);if (delta <TICK_NSEC ) - goto out; +return ; tsk->acct_timexpd = time;/*
@@ -147,8 +145,6 @@ static void __acct_update_integrals(struct task_struct *tsk,*/ tsk->acct_rss_mem1 += delta *get_mm_rss (tsk->mm) >>10 ; tsk->acct_vm_mem1 += delta * tsk->mm->total_vm >>10 ; -out: -local_irq_restore (flags); }/**
Signature · 1 line
2.5.0
Peter Zijlstra
maintainerRe: [PATCH 3/4] time,acct: drop irq save & restore from __acct_update_integrals
Quoted 30 lines · On Sun, Jan 31, 2016 at 09:12:30PM -0500, [email protected] wrote:
kernel/tsacct.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)kernel/tsacct.c +1
-3@@ -124,20 +124,18 @@ static void __acct_update_integrals(struct task_struct *tsk, cputime_t utime, cputime_t stime) {cputime_t time, dtime; -unsigned long flags;u64 delta;if (!likely (tsk->mm))return ; -local_irq_save (flags); time = stime + utime; dtime = time - tsk->acct_timexpd;/* Avoid division: cputime_t is often in nanoseconds already. */ delta =cputime_to_nsecs (dtime);if (delta <TICK_NSEC ) - goto out; +return ; tsk->acct_timexpd = time;/*
kernel/tsacct.c static void __acct_update_integrals(struct task_struct *tsk, · line 148
Quoted 6 lines
kernel/tsacct.c +0
-2@@ -147,8 +145,6 @@ static void __acct_update_integrals(struct task_struct *tsk,*/ tsk->acct_rss_mem1 += delta *get_mm_rss (tsk->mm) >>10 ; tsk->acct_vm_mem1 += delta * tsk->mm->total_vm >>10 ; -out: -local_irq_restore (flags); }
Like Frederic said on your last posting, at the very least do_exit()->acct_update_integrals() will not have IRQs disabled.
Its just the acct_account_cputime() callers that I suspect will all have IRQs disabled, but it would still be goot to verify that with a WARN_ON(!irqs_disabled()) test in there for at least one test run, and then include that you did this in the Changelog.
Re: [PATCH 3/4] time,acct: drop irq save & restore from __acct_update_integrals
Quoted 4 lines · Peter Zijlstra
Its just the
acct_account_cputime()callers that I suspect will all have IRQs disabled, but it would still be goot to verify that with a WARN_ON(!irqs_disabled()) test in there for at least one test run, and then include that you did this in the Changelog.
I verified that the WARN_ON did not trigger, but I forgot to include that in the changelog for v4
Signature · 1 line
All rights reversed
PATCH v3 4/4 sched,time: only call account_{user,sys,guest,idle}_time once a jiffy
In reply to [email protected]From: Rik van Riel <[email protected]>
After removing __acct_update_integrals from the profile, native_sched_clock remains as the top CPU user. This can be reduced by only calling account_{user,sys,guest,idle}_time once per jiffy for long running tasks on nohz_full CPUs.
This will reduce timing accuracy on nohz_full CPUs to jiffy based sampling, just like on normal CPUs. It results in totally removing native_sched_clock from the profile, and significantly speeding up the syscall entry and exit path, as well as irq entry and exit, and kvm guest entry & exit.
This code relies on another CPU advancing jiffies when the system is busy. On a nohz_full system, this is done by a housekeeping CPU.
A microbenchmark calling an invalid syscall number 10 million times in a row speeds up an additional 30% over the numbers with just the previous patches, for a total speedup of about 40% over 4.4 and 4.5-rc1.
Run times for the microbenchmark:
4.4 3.8 seconds
4.5-rc1 3.7 seconds
4.5-rc1 + first patch 3.3 seconds
4.5-rc1 + first 3 patches 3.1 seconds
4.5-rc1 + all patches 2.3 seconds
--- include/linux/sched.h | 1 + kernel/sched/cputime.c | 35 +++++++++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 6 deletions(-)
include/linux/sched.h +1 -0
@@ -1532,6 +1532,7 @@ struct task_struct {struct prev_cputime prev_cputime;#ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN seqcount_t vtime_seqcount; +unsigned long vtime_jiffies;unsigned long long vtime_snap;enum { /* Task is sleeping or running in a CPU with VTIME inactive */
kernel/sched/cputime.c +29 -6
@@ -668,6 +668,15 @@ void thread_group_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime#endif /* !CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */ #ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN +static bool vtime_jiffies_changed (struct task_struct *tsk,unsigned long now) +{ +if (tsk->vtime_jiffies == jiffies) +return false; + + tsk->vtime_jiffies = jiffies; +return true; +} +static unsigned long long vtime_delta (struct task_struct *tsk) {unsigned long long clock;
@@ -699,6 +708,9 @@ static void __vtime_account_system(struct task_struct *tsk)void vtime_account_system (struct task_struct *tsk) { +if (!vtime_jiffies_changed (tsk, jiffies)) +return ; +write_seqcount_begin (&tsk->vtime_seqcount);__vtime_account_system (tsk);write_seqcount_end (&tsk->vtime_seqcount);
@@ -707,7 +719,8 @@ void vtime_account_system(struct task_struct *tsk)void vtime_gen_account_irq_exit (struct task_struct *tsk) {write_seqcount_begin (&tsk->vtime_seqcount); -__vtime_account_system (tsk); +if (vtime_jiffies_changed (tsk, jiffies)) +__vtime_account_system (tsk);if (context_tracking_in_user ()) tsk->vtime_snap_whence =VTIME_USER ;write_seqcount_end (&tsk->vtime_seqcount);
@@ -718,16 +731,19 @@ void vtime_account_user(struct task_struct *tsk)cputime_t delta_cpu;write_seqcount_begin (&tsk->vtime_seqcount); - delta_cpu =get_vtime_delta (tsk); tsk->vtime_snap_whence =VTIME_SYS ; -account_user_time (tsk, delta_cpu,cputime_to_scaled (delta_cpu)); +if (vtime_jiffies_changed (tsk, jiffies)) { + delta_cpu =get_vtime_delta (tsk); +account_user_time (tsk, delta_cpu,cputime_to_scaled (delta_cpu)); + }write_seqcount_end (&tsk->vtime_seqcount); }void vtime_user_enter (struct task_struct *tsk) {write_seqcount_begin (&tsk->vtime_seqcount); -__vtime_account_system (tsk); +if (vtime_jiffies_changed (tsk, jiffies)) +__vtime_account_system (tsk); tsk->vtime_snap_whence =VTIME_USER ;write_seqcount_end (&tsk->vtime_seqcount); }
@@ -742,7 +758,8 @@ void vtime_guest_enter(struct task_struct *tsk)* that can thus safely catch up with a tickless delta. */ write_seqcount_begin (&tsk->vtime_seqcount); -__vtime_account_system (tsk); +if (vtime_jiffies_changed (tsk, jiffies)) +__vtime_account_system (tsk); current->flags |=PF_VCPU ;write_seqcount_end (&tsk->vtime_seqcount); }
@@ -759,8 +776,12 @@ EXPORT_SYMBOL_GPL(vtime_guest_exit);void vtime_account_idle (struct task_struct *tsk) { -cputime_t delta_cpu =get_vtime_delta (tsk); +cputime_t delta_cpu; + +if (!vtime_jiffies_changed (tsk, jiffies)) +return ; + delta_cpu =get_vtime_delta (tsk);account_idle_time (delta_cpu); }
@@ -773,6 +794,7 @@ void arch_vtime_task_switch(struct task_struct *prev)write_seqcount_begin (¤t->vtime_seqcount); current->vtime_snap_whence =VTIME_SYS ; current->vtime_snap =sched_clock_cpu (smp_processor_id ()); + current->vtime_jiffies = jiffies;write_seqcount_end (¤t->vtime_seqcount); }
@@ -784,6 +806,7 @@ void vtime_init_idle(struct task_struct *t, int cpu)write_seqcount_begin (&t->vtime_seqcount); t->vtime_snap_whence =VTIME_SYS ; t->vtime_snap =sched_clock_cpu (cpu); + t->vtime_jiffies = jiffies;write_seqcount_end (&t->vtime_seqcount);local_irq_restore (flags); }
Signature · 1 line
2.5.0
Peter Zijlstra
maintainerRe: [PATCH 4/4] sched,time: only call account_{user,sys,guest,idle}_time once a jiffy
Quoted 7 lines · On Sun, Jan 31, 2016 at 09:12:31PM -0500, [email protected] wrote:
Run times for the microbenchmark:
4.4 3.8 seconds
4.5-rc1 3.7 seconds
4.5-rc1 + first patch 3.3 seconds
4.5-rc1 + first 3 patches 3.1 seconds
4.5-rc1 + all patches 2.3 seconds
Would be good to compare that also to a !NOHZ_FULL bloated path.
Re: [PATCH 4/4] sched,time: only call account_{user,sys,guest,idle}_time once a jiffy
Quoted 10 lines · Peter Zijlstra
Quoted 7 lines · On Sun, Jan 31, 2016 at 09:12:31PM -0500, [email protected] wrote:
Run times for the microbenchmark:
4.4 3.8 seconds
4.5-rc1 3.7 seconds
4.5-rc1 + first patch 3.3 seconds
4.5-rc1 + first 3 patches 3.1 seconds
4.5-rc1 + all patches 2.3 secondsWould be good to compare that also to a !NOHZ_FULL bloated path.
On a non-nohz_full, non-housekeeping CPU, I see 1.86 seconds run time, or about 20% faster than on a nohz_full CPU with all patches applied.
Signature · 1 line
All rights reversed
PATCH 1/1 perf tooling: Add 'perf bench syscall' benchmark
In reply to [email protected]Quoted 25 lines · * [email protected] <[email protected]> wrote:
(v3: address comments raised by Frederic)
Running with
nohz_fullintroduces a fair amount of overhead. Specifically, various things that are usually done from the timer interrupt are now done at syscall, irq, and guest entry and exit times.However, some of the code that is called every single time has only ever worked at jiffy resolution. The code in
__acct_update_integralswas also doing some unnecessary calculations.Getting rid of the unnecessary calculations, without changing any of the functionality in
__acct_update_integralsgets us about an 11% win.Not calling the time statistics updating code more than once per jiffy, like is done on housekeeping CPUs and on all the CPUs of a non-nohz_full system, shaves off a further 30%.
I tested this series with a microbenchmark calling an invalid syscall number ten million times in a row, on a
nohz_fullcpu.
Quoted 7 lines
Run times for the microbenchmark:
4.4 3.8 seconds
4.5-rc1 3.7 seconds
4.5-rc1 + first patch 3.3 seconds
4.5-rc1 + first 3 patches 3.1 seconds
4.5-rc1 + all patches 2.3 seconds
Another suggestion (beyond fixing the 32-bit build ;-), could you please stick your syscall microbenchmark into 'perf bench', so that we have a standardized way of checking such numbers?
In fact I'd suggest we introduce an entirely new sub-tool for system call performance measurement - and this might be the first functionality of it.
I've attached a quick patch that is basically a copy of 'perf bench numa' and which measures getppid() performance (simple syscall where the result is not cached by glibc).
I kept the process, threading and memory allocation bits of numa.c, just in case we need them to measure more complex syscalls. Maybe we could keep the threading bits and remove the memory allocation parameters, to simplify the benchmark?
Anyway, this could be a good base to start off on.
Thanks,
Ingo
=====================>
Quoted 1 line
From
ea072c9053555e71dec5eae5bb91bd6f9cc33723Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Mon, 1 Feb 2016 08:36:31 +0100
Subject: [PATCH] perf bench: Add 'syscall' benchmark utility
Just a basic 'perf bench syscall null' variant, which measures getppid() performance.
--- tools/perf/bench/Build | 1 + tools/perf/bench/bench.h | 1 + tools/perf/bench/syscall.c | 1598 ++++++++++++++++++++++++++++++++++++++++++++ tools/perf/builtin-bench.c | 16 +- 4 files changed, 1612 insertions(+), 4 deletions(-)
tools/perf/bench/Build +1 -0
@@ -1,3 +1,4 @@ +perf-y += syscall.o perf-y += sched-messaging.o perf-y += sched-pipe.o perf-y += mem-functions.o
tools/perf/bench/bench.h +1 -0
@@ -26,6 +26,7 @@#endif extern int bench_numa (int argc,const char **argv,const char *prefix); +extern int bench_syscall (int argc,const char **argv,const char *prefix);extern int bench_sched_messaging (int argc,const char **argv,const char *prefix);extern int bench_sched_pipe (int argc,const char **argv,const char *prefix); extern int bench_mem_memcpy(int argc, const char **argv,
tools/perf/bench/syscall.c new file +1598 -0
1,598 lines not on this page · raw message
tools/perf/builtin-bench.c +12 -4
@@ -9,10 +9,11 @@/* * Available benchmark collection list: * -* sched ... scheduler and IPC performance -* mem ... memory access performance -* numa ... NUMA scheduling and MM performance -* futex ... Futex performance +* syscall ... syscall performance +* sched ... scheduler and IPC performance +* mem ... memory access performance +* numa ... NUMA scheduling and MM performance +* futex ... Futex performance */ #include "perf.h" #include "util/util.h"
@@ -33,6 +34,12 @@ struct bench {bench_fn_t fn; }; +static struct bench syscall_benchmarks[] = { + {"null" ,"Benchmark for NULL syscall workloads" , bench_syscall }, + {"all" ,"Run all syscall benchmarks" ,NULL }, + {NULL ,NULL ,NULL } +}; +#ifdef HAVE_LIBNUMA_SUPPORT static struct bench numa_benchmarks[] = { { "mem", "Benchmark for NUMA workloads", bench_numa },
@@ -73,6 +80,7 @@ struct collection { };static struct collection collections[] = { + {"syscall" ,"syscall benchmarks" , syscall_benchmarks }, {"sched" ,"Scheduler and IPC benchmarks" , sched_benchmarks }, {"mem" ,"Memory access benchmarks" , mem_benchmarks }, #ifdef HAVE_LIBNUMA_SUPPORT
PATCH 1/1 perf tooling: Simplify 'perf bench syscall'
Quoted 5 lines · Ingo Molnar
[...]
I kept the process, threading and memory allocation bits of
numa.c, just in case we need them to measure more complex syscalls. Maybe we could keep the threading bits and remove the memory allocation parameters, to simplify the benchmark?
So the patch below removes NUMA details: convergence measurement and memory access pattern details. This reduces the linecount by about 30%. Should be combined with the previous patch I suspect.
Thanks,
Ingo
==================>
Quoted 1 line
From
a992aecebe12a195ffa74e09fcbe6b48db4430e3Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Mon, 1 Feb 2016 08:46:39 +0100
Subject: [PATCH] perf tooling: Simplify 'perf bench syscall'
Remove NUMA legacies.
--- tools/perf/bench/syscall.c | 316 +-------------------------------------------- 1 file changed, 5 insertions(+), 311 deletions(-)
tools/perf/bench/syscall.c +5 -311
@@ -81,11 +81,6 @@ struct params {double mb_thread;/* Access patterns to the working set: */ -bool data_reads; -bool data_writes; -bool data_backwards; -bool data_zero_memset; -bool data_rand_walk;u32 nr_loops;u32 nr_secs;u32 sleep_usecs;
@@ -108,10 +103,6 @@ struct params {int nr_tasks;bool show_quiet; -bool show_convergence; -bool measure_convergence; - -int perturb_secs;int nr_cpus;int nr_nodes;
@@ -139,8 +130,6 @@ struct global_info {struct thread_data *threads; -/* Convergence latency measurement: */ -bool all_converged;bool stop_work;int print_once;
@@ -168,23 +157,13 @@ static const struct option options[] = {OPT_UINTEGER ('s' ,"nr_secs" , &p0.nr_secs,"max number of seconds to run (default: 5 secs)" ),OPT_UINTEGER ('u' ,"usleep" , &p0.sleep_usecs,"usecs to sleep per loop iteration" ), -OPT_BOOLEAN ('R' ,"data_reads" , &p0.data_reads,"access the data via writes (can be mixed with -W)" ), -OPT_BOOLEAN ('W' ,"data_writes" , &p0.data_writes,"access the data via writes (can be mixed with -R)" ), -OPT_BOOLEAN ('B' ,"data_backwards" , &p0.data_backwards,"access the data backwards as well" ), -OPT_BOOLEAN ('Z' ,"data_zero_memset" , &p0.data_zero_memset,"access the data via glibc bzero only" ), -OPT_BOOLEAN ('r' ,"data_rand_walk" , &p0.data_rand_walk,"access the data with random (32bit LFSR) walk" ), - -OPT_BOOLEAN ('z' ,"init_zero" , &p0.init_zero,"bzero the initial allocations" ),OPT_BOOLEAN ('I' ,"init_random" , &p0.init_random,"randomize the contents of the initial allocations" ),OPT_BOOLEAN ('0' ,"init_cpu0" , &p0.init_cpu0,"do the initial allocations on CPU#0" ), -OPT_INTEGER ('x' ,"perturb_secs" , &p0.perturb_secs,"perturb thread 0/0 every X secs, to test convergence stability" ),OPT_INCR ('d' ,"show_details" , &p0.show_details,"Show details" ),OPT_INCR ('a' ,"all" , &p0.run_all,"Run all tests in the suite" ),OPT_INTEGER ('H' ,"thp" , &p0.thp,"MADV_NOHUGEPAGE < 0 < MADV_HUGEPAGE" ), -OPT_BOOLEAN ('c' ,"show_convergence" , &p0.show_convergence,"show convergence details" ), -OPT_BOOLEAN ('m' ,"measure_convergence" , &p0.measure_convergence,"measure convergence latency" ),OPT_BOOLEAN ('q' ,"quiet" , &p0.show_quiet,"quiet mode" ), OPT_BOOLEAN('S', "serialize-startup", &p0.serialize_startup,"serialize thread startup"),
@@ -208,32 +187,6 @@ static const char * const syscall_usage[] = { NULL }; -static cpu_set_t bind_to_cpu (int target_cpu) -{ -cpu_set_t orig_mask, mask; -int ret; - - ret =sched_getaffinity (0 ,sizeof (orig_mask), &orig_mask); -BUG_ON (ret); - -CPU_ZERO (&mask); - -if (target_cpu ==-1 ) { -int cpu; - -for (cpu =0 ; cpu < g->p.nr_cpus; cpu++) -CPU_SET (cpu, &mask); - }else { -BUG_ON (target_cpu <0 || target_cpu >= g->p.nr_cpus); -CPU_SET (target_cpu, &mask); - } - - ret =sched_setaffinity (0 ,sizeof (mask), &mask); -BUG_ON (ret); - -return orig_mask; -} -static cpu_set_t bind_to_node (int target_node) {int cpus_per_node = g->p.nr_cpus/g->p.nr_nodes;
@@ -699,222 +652,11 @@ static void update_curr_cpu(int task_nr, unsigned long bytes_worked)prctl (0 , bytes_worked); } -#define MAX_NR_NODES 64 - -/* -* Count the number of nodes a process's threads -* are spread out on. -* -* A count of 1 means that the process is compressed -* to a single node. A count of g->p.nr_nodes means it's -* spread out on the whole system. -*/ -static int count_process_nodes (int process_nr) -{ -char node_present[MAX_NR_NODES ] = {0 , }; -int nodes; -int n, t; - -for (t =0 ; t < g->p.nr_threads; t++) { -struct thread_data *td; -int task_nr; -int node; - - task_nr = process_nr*g->p.nr_threads + t; - td = g->threads + task_nr; - - node =numa_node_of_cpu (td->curr_cpu); -if (node <0 )/* curr_cpu was likely still -1 */ -return 0 ; - - node_present[node] =1 ; - } - - nodes =0 ; - -for (n =0 ; n <MAX_NR_NODES ; n++) - nodes += node_present[n]; - -return nodes; -} - -/* -* Count the number of distinct process-threads a node contains. -* -* A count of 1 means that the node contains only a single -* process. If all nodes on the system contain at most one -* process then we are well-converged. -*/ -static int count_node_processes (int node) -{ -int processes =0 ; -int t, p; - -for (p =0 ; p < g->p.nr_proc; p++) { -for (t =0 ; t < g->p.nr_threads; t++) { -struct thread_data *td; -int task_nr; -int n; - - task_nr = p*g->p.nr_threads + t; - td = g->threads + task_nr; - - n =numa_node_of_cpu (td->curr_cpu); -if (n == node) { - processes++; -break ; - } - } - } - -return processes; -} - -static void calc_convergence_compression (int *strong) -{ -unsigned int nodes_min, nodes_max; -int p; - - nodes_min =-1 ; - nodes_max =0 ; - -for (p =0 ; p < g->p.nr_proc; p++) { -unsigned int nodes =count_process_nodes (p); - -if (!nodes) { - *strong =0 ; -return ; - } - - nodes_min =min (nodes, nodes_min); - nodes_max =max (nodes, nodes_max); - } - -/* Strong convergence: all threads compress on a single node: */ -if (nodes_min ==1 && nodes_max ==1 ) { - *strong =1 ; - }else { - *strong =0 ; -tprintf (" {%d-%d}" , nodes_min, nodes_max); - } -} - -static void calc_convergence (double runtime_ns_max,double *convergence) -{ -unsigned int loops_done_min, loops_done_max; -int process_groups; -int nodes[MAX_NR_NODES ]; -int distance; -int nr_min; -int nr_max; -int strong; -int sum; -int nr; -int node; -int cpu; -int t; - -if (!g->p.show_convergence && !g->p.measure_convergence) -return ; - -for (node =0 ; node < g->p.nr_nodes; node++) - nodes[node] =0 ; - - loops_done_min =-1 ; - loops_done_max =0 ; - -for (t =0 ; t < g->p.nr_tasks; t++) { -struct thread_data *td = g->threads + t; -unsigned int loops_done; - - cpu = td->curr_cpu; - -/* Not all threads have written it yet: */ -if (cpu <0 ) -continue ; - - node =numa_node_of_cpu (cpu); - - nodes[node]++; - - loops_done = td->loops_done; - loops_done_min =min (loops_done, loops_done_min); - loops_done_max =max (loops_done, loops_done_max); - } - - nr_max =0 ; - nr_min = g->p.nr_tasks; - sum =0 ; - -for (node =0 ; node < g->p.nr_nodes; node++) { - nr = nodes[node]; - nr_min =min (nr, nr_min); - nr_max =max (nr, nr_max); - sum += nr; - } -BUG_ON (nr_min > nr_max); - -BUG_ON (sum > g->p.nr_tasks); - -if (0 && (sum < g->p.nr_tasks)) -return ; - -/* -* Count the number of distinct process groups present -* on nodes - when we are converged this will decrease -* to g->p.nr_proc: -*/ - process_groups =0 ; - -for (node =0 ; node < g->p.nr_nodes; node++) { -int processes =count_node_processes (node); - - nr = nodes[node]; -tprintf (" %2d/%-2d" , nr, processes); - - process_groups += processes; - } - - distance = nr_max - nr_min; - -tprintf (" [%2d/%-2d]" , distance, process_groups); - -tprintf (" l:%3d-%-3d (%3d)" , - loops_done_min, loops_done_max, loops_done_max-loops_done_min); - -if (loops_done_min && loops_done_max) { -double skew =1.0 - (double )loops_done_min/loops_done_max; - -tprintf (" [%4.1f%%]" , skew *100.0 ); - } - -calc_convergence_compression (&strong); - -if (strong && process_groups == g->p.nr_proc) { -if (!*convergence) { - *convergence = runtime_ns_max; -tprintf (" (%6.1fs converged)\n" , *convergence/1e9 ); -if (g->p.measure_convergence) { - g->all_converged = true; - g->stop_work = true; - } - } - }else { -if (*convergence) { -tprintf (" (%6.1fs de-converged)" , runtime_ns_max/1e9 ); - *convergence =0 ; - } -tprintf ("\n" ); - } -} - -static void show_summary (double runtime_ns_max,int l,double *convergence) +static void show_summary (double runtime_ns_max,int l) {tprintf ("\r # %5.1f%% [%.1f mins]" , (double )(l+1 )/g->p.nr_loops*100.0 , runtime_ns_max/1e9 /60.0 ); -calc_convergence (runtime_ns_max, convergence); -if (g->p.show_details >=0 )fflush (stdout); }
@@ -925,11 +667,9 @@ static void *worker_thread(void *__tdata)struct timeval start0, start, stop, diff;int process_nr = td->process_nr;int thread_nr = td->thread_nr; -unsigned long last_perturbance;int task_nr = td->task_nr;int details = g->p.show_details; -int first_task, last_task; -double convergence =0 ; +int last_task;u64 val = td->val;double runtime_ns_max;u8 *global_data;
@@ -955,10 +695,6 @@ static void *worker_thread(void *__tdata)if (process_nr == g->p.nr_proc-1 && thread_nr == g->p.nr_threads-1 ) last_task =1 ; - first_task =0 ; -if (process_nr ==0 && thread_nr ==0 ) - first_task =1 ; -if (details >=2 ) {printf ("# thread %2d / %2d global mem: %p, process mem: %p, thread mem: %p\n" , process_nr, thread_nr, global_data, process_data, thread_data);
@@ -983,7 +719,6 @@ static void *worker_thread(void *__tdata)gettimeofday (&start0,NULL ); start = stop = start0; - last_perturbance = start.tv_sec;for (l =0 ; l < g->p.nr_loops; l++) { start = stop;
@@ -1015,7 +750,7 @@ static void *worker_thread(void *__tdata)update_curr_cpu (task_nr, work_done); bytes_done += work_done; -if (details <0 && !g->p.perturb_secs && !g->p.measure_convergence && !g->p.nr_secs) +if (details <0 && !g->p.nr_secs)continue ; td->loops_done = l;
@@ -1035,37 +770,6 @@ static void *worker_thread(void *__tdata)if (start.tv_sec == stop.tv_sec)continue ; -/* -* Perturb the first task's equilibrium every g->p.perturb_secs seconds, -* by migrating to CPU#0: -*/ -if (first_task && g->p.perturb_secs && (int )(stop.tv_sec - last_perturbance) >= g->p.perturb_secs) { -cpu_set_t orig_mask; -int target_cpu; -int this_cpu; - - last_perturbance = stop.tv_sec; - -/* -* Depending on where we are running, move into -* the other half of the system, to create some -* real disturbance: -*/ - this_cpu = g->threads[task_nr].curr_cpu; -if (this_cpu < g->p.nr_cpus/2 ) - target_cpu = g->p.nr_cpus-1 ; -else - target_cpu =0 ; - - orig_mask =bind_to_cpu (target_cpu); - -/* Here we are running on the target CPU already */ -if (details >=1 ) -printf (" (injecting perturbalance, moved to CPU#%d)\n" , target_cpu); - -bind_to_cpumask (orig_mask); - } -if (details >=3 ) {timersub (&stop, &start, &diff); runtime_ns_max = diff.tv_sec *1000000000 ;
@@ -1084,7 +788,7 @@ static void *worker_thread(void *__tdata) runtime_ns_max = diff.tv_sec *1000000000ULL ; runtime_ns_max += diff.tv_usec *1000ULL ; -show_summary (runtime_ns_max, l, &convergence); +show_summary (runtime_ns_max, l); }gettimeofday (&stop,NULL );
@@ -1226,8 +930,7 @@ static int init(void) g->p.nr_nodes =numa_max_node () +1 ; -/* char array in count_process_nodes(): */ -BUG_ON (g->p.nr_nodes >MAX_NR_NODES || g->p.nr_nodes <0 ); +BUG_ON (g->p.nr_nodes <0 );if (g->p.show_quiet && !g->p.show_details) g->p.show_details =-1 ;
@@ -1427,11 +1130,6 @@ static int __bench_syscall(const char *name) bytes = g->bytes_done; runtime_avg = (double )runtime_ns_sum / g->p.nr_tasks /1e9 ; -if (g->p.measure_convergence) { -print_res (name, runtime_sec_max, -"secs," ,"NUMA-convergence-latency" ,"secs latency to NUMA-converge" ); - } -print_res (name, runtime_sec_max,"secs," ,"runtime-max/thread" ,"secs slowest (max) thread-runtime" );
@@ -1517,10 +1215,6 @@ static void init_params(struct params *p, const char *name, int argc, const char/* Initialize nonzero defaults: */ p->serialize_startup =1 ; - p->data_reads = true; - p->data_writes = true; - p->data_backwards = true; - p->data_rand_walk = true; p->nr_loops =10000000 ; p->init_random = true; p->mb_global_str ="1" ;
Re: [PATCH] perf tooling: Add 'perf bench syscall' benchmark
In reply to Ingo MolnarQuoted 48 lines · Ingo Molnar
Quoted 25 lines · * [email protected] <[email protected]> wrote:
(v3: address comments raised by Frederic)
Running with
nohz_fullintroduces a fair amount of overhead. Specifically, various things that are usually done from the timer interrupt are now done at syscall, irq, and guest entry and exit times.However, some of the code that is called every single time has only ever worked at jiffy resolution. The code in
__acct_update_integralswas also doing some unnecessary calculations.Getting rid of the unnecessary calculations, without changing any of the functionality in
__acct_update_integralsgets us about an 11% win.Not calling the time statistics updating code more than once per jiffy, like is done on housekeeping CPUs and on all the CPUs of a non-nohz_full system, shaves off a further 30%.
I tested this series with a microbenchmark calling an invalid syscall number ten million times in a row, on a
nohz_fullcpu.Quoted 7 lines
Run times for the microbenchmark:
4.4 3.8 seconds
4.5-rc1 3.7 seconds4.5-rc1 + first patch 3.3 seconds 4.5-rc1 + first 3 patches 3.1 seconds 4.5-rc1 + all patches 2.3 secondsAnother suggestion (beyond fixing the 32-bit build ;-), could you please stick your syscall microbenchmark into 'perf bench', so that we have a standardized way of checking such numbers?
In fact I'd suggest we introduce an entirely new sub-tool for system call performance measurement - and this might be the first functionality of it.
I've attached a quick patch that is basically a copy of 'perf bench numa' and which measures
getppid()performance (simple syscall where the result is not cached by glibc).
Quoted 5 lines
I kept the process, threading and memory allocation bits of
numa.c, just in case we need them to measure more complex syscalls. Maybe we could keep the threading bits and remove the memory allocation parameters, to simplify the benchmark?Anyway, this could be a good base to start off on.
So much code...
I'll try to take a look this week. It shouldn't be so hard to port my rdpmc-based widget over to this.
--Andy
Re: [PATCH] perf tooling: Add 'perf bench syscall' benchmark
Quoted 49 lines · Andy Lutomirski
Quoted 48 lines · Ingo Molnar
Quoted 25 lines · * [email protected] <[email protected]> wrote:
(v3: address comments raised by Frederic)
Running with
nohz_fullintroduces a fair amount of overhead. Specifically, various things that are usually done from the timer interrupt are now done at syscall, irq, and guest entry and exit times.However, some of the code that is called every single time has only ever worked at jiffy resolution. The code in
__acct_update_integralswas also doing some unnecessary calculations.Getting rid of the unnecessary calculations, without changing any of the functionality in
__acct_update_integralsgets us about an 11% win.Not calling the time statistics updating code more than once per jiffy, like is done on housekeeping CPUs and on all the CPUs of a non-nohz_full system, shaves off a further 30%.
I tested this series with a microbenchmark calling an invalid syscall number ten million times in a row, on a
nohz_fullcpu.Quoted 7 lines
Run times for the microbenchmark:
4.4 3.8 seconds
4.5-rc1 3.7 seconds4.5-rc1 + first patch 3.3 seconds 4.5-rc1 + first 3 patches 3.1 seconds 4.5-rc1 + all patches 2.3 secondsAnother suggestion (beyond fixing the 32-bit build ;-), could you please stick your syscall microbenchmark into 'perf bench', so that we have a standardized way of checking such numbers?
In fact I'd suggest we introduce an entirely new sub-tool for system call performance measurement - and this might be the first functionality of it.
I've attached a quick patch that is basically a copy of 'perf bench numa' and which measures
getppid()performance (simple syscall where the result is not cached by glibc).
Quoted 7 lines
Quoted 5 lines
I kept the process, threading and memory allocation bits of
numa.c, just in case we need them to measure more complex syscalls. Maybe we could keep the threading bits and remove the memory allocation parameters, to simplify the benchmark?Anyway, this could be a good base to start off on.
So much code...
Arguably 90% of that should be factored out, as it's now a duplicate between bench/numa.c and bench/syscall.c.
Technically, for a minimum benchmark, something like this would already be functional for tools/perf/bench/syscall.c:
#include "../perf.h"
#include "../util/util.h"
#include "../builtin.h"
#include "bench.h"
static void run_syscall_benchmark(void)
{
[ .... your benchmark loop as-is ... ]
}int bench_syscall(int argc __maybe_unused, const char **argv __maybe_unused, const char *prefix __maybe_unused)
{
run_syscall_benchmark();
switch (bench_format) {
case BENCH_FORMAT_DEFAULT:
printf("print results in human-readable format\n");
break;
case BENCH_FORMAT_SIMPLE:
printf("print results in machine-parseable format\n");
break;
default:
BUG_ON(1);
}
return 0;
}Plus the small amount of glue for bench_sycall() I sent in the first patch.
Completely untested.
If the loop is long enough then even without any timing measurement this would be usable via:
perf stat --null --repeat 10 perf bench syscall
as the 'perf stat' will do the timing and statistics.
Quoted 2 lines
I'll try to take a look this week. It shouldn't be so hard to port my rdpmc-based widget over to this.
Sounds great to me!
Thanks,
Ingo
PATCH 1/1 perf: add 'perf bench syscall'
Quoted 96 lines · Ingo Molnar
Quoted 49 lines · Andy Lutomirski
Quoted 48 lines · Ingo Molnar
Quoted 25 lines · * [email protected] <[email protected]> wrote:
(v3: address comments raised by Frederic)
Running with
nohz_fullintroduces a fair amount of overhead. Specifically, various things that are usually done from the timer interrupt are now done at syscall, irq, and guest entry and exit times.However, some of the code that is called every single time has only ever worked at jiffy resolution. The code in
__acct_update_integralswas also doing some unnecessary calculations.Getting rid of the unnecessary calculations, without changing any of the functionality in
__acct_update_integralsgets us about an 11% win.Not calling the time statistics updating code more than once per jiffy, like is done on housekeeping CPUs and on all the CPUs of a non-nohz_full system, shaves off a further 30%.
I tested this series with a microbenchmark calling an invalid syscall number ten million times in a row, on a
nohz_fullcpu.Quoted 7 lines
Run times for the microbenchmark:
4.4 3.8 seconds
4.5-rc1 3.7 seconds4.5-rc1 + first patch 3.3 seconds 4.5-rc1 + first 3 patches 3.1 seconds 4.5-rc1 + all patches 2.3 secondsAnother suggestion (beyond fixing the 32-bit build ;-), could you please stick your syscall microbenchmark into 'perf bench', so that we have a standardized way of checking such numbers?
In fact I'd suggest we introduce an entirely new sub-tool for system call performance measurement - and this might be the first functionality of it.
I've attached a quick patch that is basically a copy of 'perf bench numa' and which measures
getppid()performance (simple syscall where the result is not cached by glibc).Quoted 7 lines
Quoted 5 lines
I kept the process, threading and memory allocation bits of
numa.c, just in case we need them to measure more complex syscalls. Maybe we could keep the threading bits and remove the memory allocation parameters, to simplify the benchmark?Anyway, this could be a good base to start off on.
So much code...
Arguably 90% of that should be factored out, as it's now a duplicate between
bench/numa.candbench/syscall.c.Technically, for a minimum benchmark, something like this would already be functional for
tools/perf/bench/syscall.c:#include "
../perf.h"
#include "../util/util.h"
#include "../builtin.h"
#include "bench.h"static void
run_syscall_benchmark(void){ [ .... your benchmark loop as-is ... ] }int
bench_syscall(int argc __maybe_unused, const char **argv __maybe_unused, const char *prefix __maybe_unused){ run_syscall_benchmark(); switch (bench_format) { case BENCH_FORMAT_DEFAULT: printf("print results in human-readable format\n"); break; case BENCH_FORMAT_SIMPLE: printf("print results in machine-parseable format\n"); break; default: BUG_ON(1); } return 0; }Plus the small amount of glue for
bench_sycall()I sent in the first patch.
Quoted 8 lines
Completely untested.
If the loop is long enough then even without any timing measurement this would be usable via:
perf stat --null --repeat 10 perf bench syscall
as the 'perf stat' will do the timing and statistics.
How about this:
--------
From: Josh Poimboeuf <[email protected]>
Subject: [PATCH] perf: add 'perf bench syscall'
Add a basic 'perf bench syscall' benchmark which does a getppid() system call in a tight loop.
--- tools/perf/bench/Build | 1 + tools/perf/bench/bench.h | 1 + tools/perf/bench/syscall.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++ tools/perf/builtin-bench.c | 18 ++++++++--- 4 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 tools/perf/bench/syscall.c
tools/perf/bench/Build +1 -0
@@ -1,5 +1,6 @@ perf-y += sched-messaging.o perf-y += sched-pipe.o +perf-y += syscall.o perf-y += mem-functions.o perf-y += futex-hash.o perf-y += futex-wake.o
tools/perf/bench/bench.h +1 -0
@@ -28,6 +28,7 @@int bench_numa (int argc,const char **argv,const char *prefix);int bench_sched_messaging (int argc,const char **argv,const char *prefix);int bench_sched_pipe (int argc,const char **argv,const char *prefix); +int bench_syscall_basic (int argc,const char **argv,const char *prefix);int bench_mem_memcpy (int argc,const char **argv,const char *prefix);int bench_mem_memset (int argc,const char **argv,const char *prefix);int bench_futex_hash (int argc,const char **argv,const char *prefix);
tools/perf/bench/syscall.c new file +80 -0
80 lines not on this page · raw message
tools/perf/builtin-bench.c +13 -5
@@ -9,10 +9,11 @@/* * Available benchmark collection list: * -* sched ... scheduler and IPC performance -* mem ... memory access performance -* numa ... NUMA scheduling and MM performance -* futex ... Futex performance +* sched ... Scheduler and IPC performance +* syscall ... System call performance +* mem ... Memory access performance +* numa ... NUMA scheduling and MM performance +* futex ... Futex performance */ #include "perf.h" #include "util/util.h"
@@ -44,10 +45,16 @@ static struct bench numa_benchmarks[] = {static struct bench sched_benchmarks[] = { {"messaging" ,"Benchmark for scheduling and IPC" , bench_sched_messaging }, {"pipe" ,"Benchmark for pipe() between two processes" , bench_sched_pipe }, - {"all" ,"Run all scheduler benchmarks" ,NULL }, + {"all" ,"Run all scheduler benchmarks" ,NULL }, {NULL ,NULL ,NULL } }; +static struct bench syscall_benchmarks[] = { + {"basic" ,"Benchmark for basic getppid() system calls" , bench_syscall_basic }, + {"all" ,"Run all syscall benchmarks" ,NULL }, + {NULL ,NULL ,NULL }, +}; +static struct bench mem_benchmarks[] = { {"memcpy" ,"Benchmark for memcpy() functions" , bench_mem_memcpy }, {"memset" ,"Benchmark for memset() functions" , bench_mem_memset },
@@ -74,6 +81,7 @@ struct collection {static struct collection collections[] = { {"sched" ,"Scheduler and IPC benchmarks" , sched_benchmarks }, + {"syscall" ,"System call benchmarks" , syscall_benchmarks }, {"mem" ,"Memory access benchmarks" , mem_benchmarks }, #ifdef HAVE_LIBNUMA_SUPPORT { "numa", "NUMA scheduling and MM benchmarks", numa_benchmarks },
Signature · 1 line
2.4.11
Re: [PATCH] perf: add 'perf bench syscall'
Quoted 7 lines · Josh Poimboeuf
From: Josh Poimboeuf <[email protected]>
Subject: [PATCH] perf: add 'perf bench syscall'Add a basic 'perf bench syscall' benchmark which does a
getppid()system call in a tight loop.
My one suggestion would be to use a different syscall than getppid(), as getppid() can have unusual performance characteristics on kernels with pid namespaces enabled. I'm a fan of prctl(-1, 0, 0, 0, 0).
Re: [PATCH] perf: add 'perf bench syscall'
Quoted 6 lines · Andy Lutomirski
Quoted 4 lines · Josh Poimboeuf
From: Josh Poimboeuf <[email protected]>
Subject: [PATCH] perf: add 'perf bench syscall'
Quoted 7 lines
Quoted 3 lines
Add a basic 'perf bench syscall' benchmark which does a
getppid()system call in a tight loop.My one suggestion would be to use a different syscall than
getppid(), asgetppid()can have unusual performance characteristics on kernels with pid namespaces enabled. I'm a fan ofprctl(-1, 0, 0, 0, 0).
Hm, can you elaborate on the unusual performance characteristics of getppid()? The code seems pretty minimal.
prctl() actually seems much worse to me, because of all the security module cruft.
Signature · 1 line
Josh
Re: [PATCH] perf: add 'perf bench syscall'
Quoted 12 lines · Josh Poimboeuf
Quoted 10 lines · Andy Lutomirski
Quoted 7 lines · Josh Poimboeuf
From: Josh Poimboeuf <[email protected]>
Subject: [PATCH] perf: add 'perf bench syscall'Add a basic 'perf bench syscall' benchmark which does a
getppid()system call in a tight loop.
Quoted 6 lines
Quoted 3 lines
My one suggestion would be to use a different syscall than
getppid(), asgetppid()can have unusual performance characteristics on kernels with pid namespaces enabled. I'm a fan ofprctl(-1, 0, 0, 0, 0).Hm, can you elaborate on the unusual performance characteristics of
getppid()? The code seems pretty minimal.
task_this_vnr -> pid_vnr -> pid_nr_ns
There's probably something better than prctl, though.
--Andy