Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1322153 > unrolled thread

[PATCH 0/2] sched,time: reduce nohz_full syscall overhead 40%

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] sched,time: reduce nohz_full syscall overhead 40% riel@redhat.com - 2016-01-29 23:50 +0100
    [PATCH 1/2] sched,time: remove pointless divides from __acct_update_integrals riel@redhat.com - 2016-01-29 23:50 +0100
      Re: [PATCH 1/2] sched,time: remove pointless divides from  __acct_update_integrals Peter Zijlstra <peterz@infradead.org> - 2016-01-30 00:20 +0100
        Re: [PATCH 1/2] sched,time: remove pointless divides from  __acct_update_integrals Frederic Weisbecker <fweisbec@gmail.com> - 2016-01-30 04:40 +0100
          Re: [PATCH 1/2] sched,time: remove pointless divides from  __acct_update_integrals Rik van Riel <riel@redhat.com> - 2016-01-30 04:50 +0100

#1322153 — [PATCH 0/2] sched,time: reduce nohz_full syscall overhead 40%

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 a 10% win.

Not calling __acct_update_integrals and related code unless
jiffies changed (__acct_update_integrals does not do anything
with smaller time intervals anyway) shaves off a further 30%.

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 + both patches			2.3 seconds

[toc] | [next] | [standalone]

#1322158 — [PATCH 1/2] sched,time: remove pointless divides from __acct_update_integrals

From: Rik van Riel <riel@redhat.com>

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 11%, with 10 million calls to an
invalid syscall number dropping from 3.7 to 3.3 seconds.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 kernel/tsacct.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 975cb49e32bf..afb5cf8ecc5f 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -125,22 +125,22 @@ 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;
+		u64 delta, usecs;
 
 		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;
+		delta = cputime_to_jiffies(dtime);
 
 		if (delta == 0)
 			goto out;
+
+		usecs = jiffies_to_usecs(delta);
+
 		tsk->acct_timexpd = time;
-		tsk->acct_rss_mem1 += delta * get_mm_rss(tsk->mm);
-		tsk->acct_vm_mem1 += delta * tsk->mm->total_vm;
+		tsk->acct_rss_mem1 += usecs * get_mm_rss(tsk->mm);
+		tsk->acct_vm_mem1 += usecs * tsk->mm->total_vm;
 	out:
 		local_irq_restore(flags);
 	}
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1322168 — Re: [PATCH 1/2] sched,time: remove pointless divides from __acct_update_integrals

On Fri, Jan 29, 2016 at 05:22:59PM -0500, riel@redhat.com wrote:
> From: Rik van Riel <riel@redhat.com>
> 
> 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 11%, with 10 million calls to an
> invalid syscall number dropping from 3.7 to 3.3 seconds.

WTH is this taskstat crap anyway? Who uses it and can't we kill it?

There seems to be an endless reserve of accounting crap.

> +++ b/kernel/tsacct.c
> @@ -125,22 +125,22 @@ static void __acct_update_integrals(struct task_struct *tsk,
>  {
>  	if (likely(tsk->mm)) {
>  		cputime_t time, dtime;
>  		unsigned long flags;
> +		u64 delta, usecs;
>  
>  		local_irq_save(flags);
>  		time = stime + utime;
>  		dtime = time - tsk->acct_timexpd;
> +		delta = cputime_to_jiffies(dtime);
>  
>  		if (delta == 0)
>  			goto out;

> +
> +		usecs = jiffies_to_usecs(delta);
> +
>  		tsk->acct_timexpd = time;
> +		tsk->acct_rss_mem1 += usecs * get_mm_rss(tsk->mm);
> +		tsk->acct_vm_mem1 += usecs * tsk->mm->total_vm;
>  	out:
>  		local_irq_restore(flags);
>  	}

Hurch, what horrible code.

Can't you at least drop the pointless indent level while you're there
anyway?

Also, it looks like the callpath through acct_account_cputime() should
already guarantee IRQs are disabled, so if you move the irq_save /
irq_restore business into acct_update_integrals() you can remove them
too from the fast path.

And I think you can kill that last divide too, if you do something like:


	nsecs = cputime_to_nsecs(dtime);
	if (nsecs < TICK_NSEC)
		goto out;

	usecs = nsecs >> 10;

	...

And if people really care, you can correct the 1024 != 1000 thing in
xacct_add_tsk() which seems to be the consumer side and thus should be
exceedingly rare.

[toc] | [prev] | [next] | [standalone]

#1322243 — Re: [PATCH 1/2] sched,time: remove pointless divides from __acct_update_integrals

On Sat, Jan 30, 2016 at 12:10:18AM +0100, Peter Zijlstra wrote:
> On Fri, Jan 29, 2016 at 05:22:59PM -0500, riel@redhat.com wrote:
> > From: Rik van Riel <riel@redhat.com>
> > 
> > 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 11%, with 10 million calls to an
> > invalid syscall number dropping from 3.7 to 3.3 seconds.
> 
> WTH is this taskstat crap anyway? Who uses it and can't we kill it?

I have no idea what it's used for, it seems to be related to taskstats
over netlink. I'm not even sure if it's actually used. There don't seem
to be a runtime offcase and I bet distros enable it. So that stuff does
some work every millisecond on millions of machines while it probably
has very few users.

SGI introduced it in 2006 and it seems that their last contribution there is
in 2008. The rest is kernel maintainance and fixes.

If there are still users of it, then at least we should disable it on runtime by
default.

[toc] | [prev] | [next] | [standalone]

#1322253 — Re: [PATCH 1/2] sched,time: remove pointless divides from __acct_update_integrals

On 01/29/2016 10:36 PM, Frederic Weisbecker wrote:
> On Sat, Jan 30, 2016 at 12:10:18AM +0100, Peter Zijlstra wrote:
>> On Fri, Jan 29, 2016 at 05:22:59PM -0500, riel@redhat.com wrote:
>>> From: Rik van Riel <riel@redhat.com>
>>>
>>> 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 11%, with 10 million calls to an
>>> invalid syscall number dropping from 3.7 to 3.3 seconds.
>>
>> WTH is this taskstat crap anyway? Who uses it and can't we kill it?
> 
> I have no idea what it's used for, it seems to be related to taskstats
> over netlink. I'm not even sure if it's actually used. There don't seem
> to be a runtime offcase and I bet distros enable it. So that stuff does
> some work every millisecond on millions of machines while it probably
> has very few users.
> 
> SGI introduced it in 2006 and it seems that their last contribution there is
> in 2008. The rest is kernel maintainance and fixes.
> 
> If there are still users of it, then at least we should disable it on runtime by
> default.

With all the non-power-of-2 divides removed, __acct_update_integrals
disappears from the profile, even running many times per millisecond.

At that point, native_sched_clock takes over the top of the profile,
and the only way I can think of getting rid of that one is making
sure it is not called twice for every syscall, irq, and kvm guest
entry/exit.

-- 
All rights reversed

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web