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

Groups > linux.kernel > #1322151 > unrolled thread

[PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy

Started byriel@redhat.com
First post2016-01-29 23:50 +0100
Last post2016-01-31 06:40 +0100
Articles 7 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy riel@redhat.com - 2016-01-29 23:50 +0100
    Re: [PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy Rik van Riel <riel@redhat.com> - 2016-01-29 23:50 +0100
      Re: [PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy Frederic Weisbecker <fweisbec@gmail.com> - 2016-01-30 15:30 +0100
        Re: [PATCH 2/2] sched,time: call __acct_update_integrals once a  jiffy Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-30 19:00 +0100
          Re: [PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy Frederic Weisbecker <fweisbec@gmail.com> - 2016-01-30 21:40 +0100
            Re: [PATCH 2/2] sched,time: call __acct_update_integrals once a  jiffy Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-31 04:00 +0100
              Re: [PATCH 2/2] sched,time: call __acct_update_integrals once a  jiffy Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-31 06:40 +0100

#1322151 — [PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy

Fromriel@redhat.com
Date2016-01-29 23:50 +0100
Subject[PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy
Message-ID<qWq1z-1yb-1@gated-at.bofh.it>
From: Rik van Riel <riel@redhat.com>

Because __acct_update_integrals does nothing unless the time
interval in question exceeds a jiffy, there is no real reason
to call it more than once a jiffy from the syscall, irq, and
guest entry & exit paths.

If tasks get rescheduled frequently, the scheduler will still
update their time statistics normally. This patch only impacts
longer running tasks.

This speeds up

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 include/linux/sched.h  |  1 +
 kernel/sched/cputime.c | 35 +++++++++++++++++++++++++++++------
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a10494a94cc3..019c3af98503 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -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 */
diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index b2ab2ffb1adc..923c110319b1 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -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(&current->vtime_seqcount);
 	current->vtime_snap_whence = VTIME_SYS;
 	current->vtime_snap = sched_clock_cpu(smp_processor_id());
+	current->vtime_jiffies = jiffies;
 	write_seqcount_end(&current->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);
 }
-- 
2.5.0

[toc] | [next] | [standalone]

#1322159

FromRik van Riel <riel@redhat.com>
Date2016-01-29 23:50 +0100
Message-ID<qWq1B-1yb-43@gated-at.bofh.it>
In reply to#1322151
On 01/29/2016 05:23 PM, riel@redhat.com wrote:
> From: Rik van Riel <riel@redhat.com>

> This speeds up

... ok, that changelog got truncated :(

Here is the full version:


Because __acct_update_integrals does nothing unless the time
interval in question exceeds a jiffy, there is no real reason
to call it more than once a jiffy from the syscall, irq, and
guest entry & exit paths.

If tasks get rescheduled frequently, the scheduler will still
update their time statistics normally.

However, longer running tasks with frequent syscall, irq,
or guest entry & exit see a difference with this patch.

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


-- 
All rights reversed

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

#1322373

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2016-01-30 15:30 +0100
Message-ID<qWEHg-4rE-1@gated-at.bofh.it>
In reply to#1322159
On Fri, Jan 29, 2016 at 05:43:28PM -0500, Rik van Riel wrote:
> On 01/29/2016 05:23 PM, riel@redhat.com wrote:
> > From: Rik van Riel <riel@redhat.com>
> 
> > This speeds up
> 
> ... ok, that changelog got truncated :(
> 
> Here is the full version:
> 
> 
> Because __acct_update_integrals does nothing unless the time
> interval in question exceeds a jiffy, there is no real reason
> to call it more than once a jiffy from the syscall, irq, and
> guest entry & exit paths.
> 
> If tasks get rescheduled frequently, the scheduler will still
> update their time statistics normally.
> 
> However, longer running tasks with frequent syscall, irq,
> or guest entry & exit see a difference with this patch.
> 
> 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 patch, 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 + both patches          2.3 seconds

Very nice improvement!

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

#1322474 — Re: [PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy

FromMike Galbraith <umgwanakikbuti@gmail.com>
Date2016-01-30 19:00 +0100
SubjectRe: [PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy
Message-ID<qWHYv-7qu-15@gated-at.bofh.it>
In reply to#1322373
On Sat, 2016-01-30 at 15:20 +0100, Frederic Weisbecker wrote:
> On Fri, Jan 29, 2016 at 05:43:28PM -0500, Rik van Riel 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 + both patches          2.3 seconds
> 
> Very nice improvement!

Tasty indeed.

When nohz_full CPUs are not isolated, ie are being used as generic
CPUs, get_nohz_timer_target() is a problem with things like tbench.

tbench 8 with Rik's patches applied:
nohz_full=empty
Throughput 3204.69 MB/sec  1.000
nohz_full=1-3,5-7 
Throughput 1354.99 MB/sec   .422  1.000
nohz_full=1-3,5-7 + club below 
Throughput 2762.22 MB/sec   .861  2.038

With Rik's patches and a club, tbench becomes nearly acceptable.
---
 include/linux/tick.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -184,7 +184,7 @@ static inline const struct cpumask *hous
 static inline bool is_housekeeping_cpu(int cpu)
 {
 #ifdef CONFIG_NO_HZ_FULL
-	if (tick_nohz_full_enabled())
+	if (tick_nohz_full_enabled() && runqueue_is_isolated(cpu))
 		return cpumask_test_cpu(cpu, housekeeping_mask);
 #endif
 	return true;

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

#1322506

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2016-01-30 21:40 +0100
Message-ID<qWKtk-Mu-7@gated-at.bofh.it>
In reply to#1322474
On Sat, Jan 30, 2016 at 06:53:05PM +0100, Mike Galbraith wrote:
> On Sat, 2016-01-30 at 15:20 +0100, Frederic Weisbecker wrote:
> > On Fri, Jan 29, 2016 at 05:43:28PM -0500, Rik van Riel 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 + both patches          2.3 seconds
> > 
> > Very nice improvement!
> 
> Tasty indeed.
> 
> When nohz_full CPUs are not isolated, ie are being used as generic
> CPUs, get_nohz_timer_target() is a problem with things like tbench.

So by isolated CPU you mean those part of isolcpus= boot option, right?

> 
> tbench 8 with Rik's patches applied:
> nohz_full=empty
> Throughput 3204.69 MB/sec  1.000
> nohz_full=1-3,5-7 
> Throughput 1354.99 MB/sec   .422  1.000
> nohz_full=1-3,5-7 + club below 
> Throughput 2762.22 MB/sec   .861  2.038
> 
> With Rik's patches and a club, tbench becomes nearly acceptable.
> ---
>  include/linux/tick.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/include/linux/tick.h
> +++ b/include/linux/tick.h
> @@ -184,7 +184,7 @@ static inline const struct cpumask *hous
>  static inline bool is_housekeeping_cpu(int cpu)
>  {
>  #ifdef CONFIG_NO_HZ_FULL
> -	if (tick_nohz_full_enabled())
> +	if (tick_nohz_full_enabled() && runqueue_is_isolated(cpu))
>  		return cpumask_test_cpu(cpu, housekeeping_mask);

This makes me confused. How forcing timers to CPUs in isolcpus is making
better results?

>  #endif
>  	return true;

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

#1322563 — Re: [PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy

FromMike Galbraith <umgwanakikbuti@gmail.com>
Date2016-01-31 04:00 +0100
SubjectRe: [PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy
Message-ID<qWQp3-51d-5@gated-at.bofh.it>
In reply to#1322506
On Sat, 2016-01-30 at 21:36 +0100, Frederic Weisbecker wrote:
> On Sat, Jan 30, 2016 at 06:53:05PM +0100, Mike Galbraith wrote:
> > On Sat, 2016-01-30 at 15:20 +0100, Frederic Weisbecker wrote:
> > > On Fri, Jan 29, 2016 at 05:43:28PM -0500, Rik van Riel 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 + both patches          2.3 seconds
> > > 
> > > Very nice improvement!
> > 
> > Tasty indeed.
> > 
> > When nohz_full CPUs are not isolated, ie are being used as generic
> > CPUs, get_nohz_timer_target() is a problem with things like tbench.
> 
> So by isolated CPU you mean those part of isolcpus= boot option,
> right?

Yes, isolated in the scheduler sense, either via isolcpus= or cpusets. 
 If the CPU is part of a scheduler domain, it is by definition part of
the generic work crew.
 
> > tbench 8 with Rik's patches applied:
> > nohz_full=empty
> > Throughput 3204.69 MB/sec  1.000
> > nohz_full=1-3,5-7 
> > Throughput 1354.99 MB/sec   .422  1.000
> > nohz_full=1-3,5-7 + club below 
> > Throughput 2762.22 MB/sec   .861  2.038
> > 
> > With Rik's patches and a club, tbench becomes nearly acceptable.
> > ---
> >  include/linux/tick.h |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > --- a/include/linux/tick.h
> > +++ b/include/linux/tick.h
> > @@ -184,7 +184,7 @@ static inline const struct cpumask *hous
> >  static inline bool is_housekeeping_cpu(int cpu)
> >  {
> >  #ifdef CONFIG_NO_HZ_FULL
> > -	if (tick_nohz_full_enabled())
> > +	if (tick_nohz_full_enabled() && runqueue_is_isolated(cpu))
> >  		return cpumask_test_cpu(cpu, housekeeping_mask);
> 
> This makes me confused. How forcing timers to CPUs in isolcpus is making
> better results?

It doesn't, it's shutting get_nohz_timer_target() down for those
nohz_full CPUs that are NOT currently isolated, are thus generic CPUs
with the capability to _become_ elite solo artists on demand.

	-Mike

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

#1322576 — Re: [PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy

FromMike Galbraith <umgwanakikbuti@gmail.com>
Date2016-01-31 06:40 +0100
SubjectRe: [PATCH 2/2] sched,time: call __acct_update_integrals once a jiffy
Message-ID<qWSTU-782-5@gated-at.bofh.it>
In reply to#1322563
On Sun, 2016-01-31 at 03:52 +0100, Mike Galbraith wrote:
> On Sat, 2016-01-30 at 21:36 +0100, Frederic Weisbecker wrote:
> > On Sat, Jan 30, 2016 at 06:53:05PM +0100, Mike Galbraith wrote:
> > > On Sat, 2016-01-30 at 15:20 +0100, Frederic Weisbecker wrote:
> > > > On Fri, Jan 29, 2016 at 05:43:28PM -0500, Rik van Riel 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 + both patches          2.3 seconds
> > > > 
> > > > Very nice improvement!
> > > 
> > > Tasty indeed.
> > > 
> > > When nohz_full CPUs are not isolated, ie are being used as
> > > generic
> > > CPUs, get_nohz_timer_target() is a problem with things like
> > > tbench.
> > 
> > So by isolated CPU you mean those part of isolcpus= boot option,
> > right?
> 
> Yes, isolated in the scheduler sense, either via isolcpus= or
> cpusets. 
>  If the CPU is part of a scheduler domain, it is by definition part
> of
> the generic work crew.
>  
> > > tbench 8 with Rik's patches applied:
> > > nohz_full=empty
> > > Throughput 3204.69 MB/sec  1.000
> > > nohz_full=1-3,5-7 
> > > Throughput 1354.99 MB/sec   .422  1.000
> > > nohz_full=1-3,5-7 + club below 
> > > Throughput 2762.22 MB/sec   .861  2.038
> > > 
> > > With Rik's patches and a club, tbench becomes nearly acceptable.
> > > ---
> > >  include/linux/tick.h |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > --- a/include/linux/tick.h
> > > +++ b/include/linux/tick.h
> > > @@ -184,7 +184,7 @@ static inline const struct cpumask *hous
> > >  static inline bool is_housekeeping_cpu(int cpu)
> > >  {
> > >  #ifdef CONFIG_NO_HZ_FULL
> > > -	if (tick_nohz_full_enabled())
> > > +	if (tick_nohz_full_enabled() &&
> > > runqueue_is_isolated(cpu))
> > >  		return cpumask_test_cpu(cpu, housekeeping_mask);
> > 
> > This makes me confused. How forcing timers to CPUs in isolcpus is
> > making
> > better results?
> 
> It doesn't, it's shutting get_nohz_timer_target() down for those
> nohz_full CPUs that are NOT currently isolated, are thus generic CPUs
> with the capability to _become_ elite solo artists on demand.

Damn, I'm gonna have to ask...

If an isolated elite task sets the alarm, why would it want it to go
off somewhere else, maybe in the middle of a death metal concert? 
 Seems it must induce jitter.  Why is this a good thing?

	-Mike

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web