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

Groups > linux.kernel > #1247088 > unrolled thread

[PATCH v2 0/4] timer: Improve itimers scalability

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/4] timer: Improve itimers scalability Jason Low <jason.low2@hp.com> - 2015-10-14 21:10 +0200
    [PATCH v2 3/4] timer: Convert cputimer->running to bool Jason Low <jason.low2@hp.com> - 2015-10-14 21:10 +0200
      [tip:timers/core] posix_cpu_timer: Convert cputimer->  running to bool tip-bot for Jason Low <tipbot@zytor.com> - 2015-10-15 11:30 +0200
    Re: [PATCH v2 0/4] timer: Improve itimers scalability "George Spelvin" <linux@horizon.com> - 2015-10-14 23:20 +0200
      Re: [PATCH v2 0/4] timer: Improve itimers scalability Frederic Weisbecker <fweisbec@gmail.com> - 2015-10-15 14:40 +0200
      Re: [PATCH v2 0/4] timer: Improve itimers scalability Jason Low <jason.low@hpe.com> - 2015-10-15 21:10 +0200
    Re: [PATCH v2 0/4] timer: Improve itimers scalability Ingo Molnar <mingo@kernel.org> - 2015-10-15 10:50 +0200
      Re: [PATCH v2 0/4] timer: Improve itimers scalability Jason Low <jason.low@hpe.com> - 2015-10-15 21:10 +0200
        Re: [PATCH v2 0/4] timer: Improve itimers scalability Ingo Molnar <mingo@kernel.org> - 2015-10-16 09:20 +0200
          Re: [PATCH v2 0/4] timer: Improve itimers scalability Jason Low <jason.low@hpe.com> - 2015-10-16 19:40 +0200
            Re: [PATCH v2 0/4] timer: Improve itimers scalability Hideaki Kimura <hideaki.kimura@hpe.com> - 2015-10-16 20:10 +0200

#1247088 — [PATCH v2 0/4] timer: Improve itimers scalability

While running a database workload on a 16 socket machine, there were
scalability issues related to itimers. The following link contains a
more detailed summary of the issues at the application level.

https://lkml.org/lkml/2015/8/26/737

Commit 1018016c706f addressed the issue with the thread_group_cputimer
spinlock taking up a significant portion of total run time.
This patch series addresses the secondary issue where a lot of time is
spent trying to acquire the sighand lock. It was found in some cases
that 200+ threads were simultaneously contending for the same sighand
lock, reducing throughput by more than 30%.

With this patch set (along with commit 1018016c706f mentioned above),
the performance hit of itimers almost completely goes away on the
16 socket system.

Jason Low (4):
  timer: Optimize fastpath_timer_check()
  timer: Check thread timers only when there are active thread timers
  timer: Convert cputimer->running to bool
  timer: Reduce unnecessary sighand lock contention

 include/linux/init_task.h      |    3 +-
 include/linux/sched.h          |    9 ++++--
 kernel/fork.c                  |    2 +-
 kernel/time/posix-cpu-timers.c |   63 ++++++++++++++++++++++++++++-----------
 4 files changed, 54 insertions(+), 23 deletions(-)

-- 
1.7.2.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1247092 — [PATCH v2 3/4] timer: Convert cputimer->running to bool

In the next patch in this series, a new field 'checking_timer' will
be added to 'struct thread_group_cputimer'. Both this and the
existing 'running' integer field are just used as boolean values. To
save space in the structure, we can make both of these fields booleans.

This is a preparatory patch to convert the existing running integer
field to a boolean.

Suggested-by: George Spelvin <linux@horizon.com>
Signed-off-by: Jason Low <jason.low2@hp.com>
---
 include/linux/init_task.h      |    2 +-
 include/linux/sched.h          |    6 +++---
 kernel/fork.c                  |    2 +-
 kernel/time/posix-cpu-timers.c |    4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index e38681f..c43b80f 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -59,7 +59,7 @@ extern struct fs_struct init_fs;
 	.rlim		= INIT_RLIMITS,					\
 	.cputimer	= { 						\
 		.cputime_atomic	= INIT_CPUTIME_ATOMIC,			\
-		.running	= 0,					\
+		.running	= false,				\
 	},								\
 	INIT_PREV_CPUTIME(sig)						\
 	.cred_guard_mutex =						\
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 23ca455..35a9c46 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -623,15 +623,15 @@ struct task_cputime_atomic {
 /**
  * struct thread_group_cputimer - thread group interval timer counts
  * @cputime_atomic:	atomic thread group interval timers.
- * @running:		non-zero when there are timers running and
- *			@cputime receives updates.
+ * @running:		true when there are timers running and
+ *			@cputime_atomic receives updates.
  *
  * This structure contains the version of task_cputime, above, that is
  * used for thread group CPU timer calculations.
  */
 struct thread_group_cputimer {
 	struct task_cputime_atomic cputime_atomic;
-	int running;
+	bool running;
 };
 
 #include <linux/rwsem.h>
diff --git a/kernel/fork.c b/kernel/fork.c
index 83dea82..fe3f371 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1101,7 +1101,7 @@ static void posix_cpu_timers_init_group(struct signal_struct *sig)
 	cpu_limit = READ_ONCE(sig->rlim[RLIMIT_CPU].rlim_cur);
 	if (cpu_limit != RLIM_INFINITY) {
 		sig->cputime_expires.prof_exp = secs_to_cputime(cpu_limit);
-		sig->cputimer.running = 1;
+		sig->cputimer.running = true;
 	}
 
 	/* The timer lists. */
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 6f6e252..2d58153 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -249,7 +249,7 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
 		 * but barriers are not required because update_gt_cputime()
 		 * can handle concurrent updates.
 		 */
-		WRITE_ONCE(cputimer->running, 1);
+		WRITE_ONCE(cputimer->running, true);
 	}
 	sample_cputime_atomic(times, &cputimer->cputime_atomic);
 }
@@ -918,7 +918,7 @@ static inline void stop_process_timers(struct signal_struct *sig)
 	struct thread_group_cputimer *cputimer = &sig->cputimer;
 
 	/* Turn off cputimer->running. This is done without locking. */
-	WRITE_ONCE(cputimer->running, 0);
+	WRITE_ONCE(cputimer->running, false);
 }
 
 static u32 onecputick;
-- 
1.7.2.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1247610 — [tip:timers/core] posix_cpu_timer: Convert cputimer-> running to bool

Commit-ID:  d5c373eb5610686162ff50429f63f4c00c554799
Gitweb:     http://git.kernel.org/tip/d5c373eb5610686162ff50429f63f4c00c554799
Author:     Jason Low <jason.low2@hp.com>
AuthorDate: Wed, 14 Oct 2015 12:07:55 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 15 Oct 2015 11:23:41 +0200

posix_cpu_timer: Convert cputimer->running to bool

In the next patch in this series, a new field 'checking_timer' will
be added to 'struct thread_group_cputimer'. Both this and the
existing 'running' integer field are just used as boolean values. To
save space in the structure, we can make both of these fields booleans.

This is a preparatory patch to convert the existing running integer
field to a boolean.

Suggested-by: George Spelvin <linux@horizon.com>
Signed-off-by: Jason Low <jason.low2@hp.com>
Reviewed: George Spelvin <linux@horizon.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: hideaki.kimura@hpe.com
Cc: terry.rudd@hpe.com
Cc: scott.norton@hpe.com
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1444849677-29330-4-git-send-email-jason.low2@hp.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/init_task.h      | 2 +-
 include/linux/sched.h          | 6 +++---
 kernel/fork.c                  | 2 +-
 kernel/time/posix-cpu-timers.c | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index e38681f..c43b80f 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -59,7 +59,7 @@ extern struct fs_struct init_fs;
 	.rlim		= INIT_RLIMITS,					\
 	.cputimer	= { 						\
 		.cputime_atomic	= INIT_CPUTIME_ATOMIC,			\
-		.running	= 0,					\
+		.running	= false,				\
 	},								\
 	INIT_PREV_CPUTIME(sig)						\
 	.cred_guard_mutex =						\
diff --git a/include/linux/sched.h b/include/linux/sched.h
index b7b9501..6c8504a 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -617,15 +617,15 @@ struct task_cputime_atomic {
 /**
  * struct thread_group_cputimer - thread group interval timer counts
  * @cputime_atomic:	atomic thread group interval timers.
- * @running:		non-zero when there are timers running and
- * 			@cputime receives updates.
+ * @running:		true when there are timers running and
+ *			@cputime_atomic receives updates.
  *
  * This structure contains the version of task_cputime, above, that is
  * used for thread group CPU timer calculations.
  */
 struct thread_group_cputimer {
 	struct task_cputime_atomic cputime_atomic;
-	int running;
+	bool running;
 };
 
 #include <linux/rwsem.h>
diff --git a/kernel/fork.c b/kernel/fork.c
index 2845623..6ac8942 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1101,7 +1101,7 @@ static void posix_cpu_timers_init_group(struct signal_struct *sig)
 	cpu_limit = READ_ONCE(sig->rlim[RLIMIT_CPU].rlim_cur);
 	if (cpu_limit != RLIM_INFINITY) {
 		sig->cputime_expires.prof_exp = secs_to_cputime(cpu_limit);
-		sig->cputimer.running = 1;
+		sig->cputimer.running = true;
 	}
 
 	/* The timer lists. */
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 6f6e252..2d58153 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -249,7 +249,7 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
 		 * but barriers are not required because update_gt_cputime()
 		 * can handle concurrent updates.
 		 */
-		WRITE_ONCE(cputimer->running, 1);
+		WRITE_ONCE(cputimer->running, true);
 	}
 	sample_cputime_atomic(times, &cputimer->cputime_atomic);
 }
@@ -918,7 +918,7 @@ static inline void stop_process_timers(struct signal_struct *sig)
 	struct thread_group_cputimer *cputimer = &sig->cputimer;
 
 	/* Turn off cputimer->running. This is done without locking. */
-	WRITE_ONCE(cputimer->running, 0);
+	WRITE_ONCE(cputimer->running, false);
 }
 
 static u32 onecputick;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1247192

I'm going to give 4/4 a closer look to see if the races with timer
expiration make more sense to me than last time around.
(E.g. do CPU time signals even work in CONFIG_NO_HZ_FULL?)

But although I haven't yet convinced myself the current code is right,
the changes don't seem to make it any worse.  So consider all four

Reviewed-by: George Spelvin <linux@horizon.com>

Thank you!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1247766

On Wed, Oct 14, 2015 at 05:18:27PM -0400, George Spelvin wrote:
> I'm going to give 4/4 a closer look to see if the races with timer
> expiration make more sense to me than last time around.
> (E.g. do CPU time signals even work in CONFIG_NO_HZ_FULL?)

Those enqueued with timer_settime() do work. But itimers,
and rlimits (RLIMIT_RTTIME, RLIMIT_CPU) aren't supported well. I
need to rework that.

> 
> But although I haven't yet convinced myself the current code is right,
> the changes don't seem to make it any worse.  So consider all four
> 
> Reviewed-by: George Spelvin <linux@horizon.com>
> 
> Thank you!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1248090

On Wed, 2015-10-14 at 17:18 -0400, George Spelvin wrote:
> I'm going to give 4/4 a closer look to see if the races with timer
> expiration make more sense to me than last time around.
> (E.g. do CPU time signals even work in CONFIG_NO_HZ_FULL?)
> 
> But although I haven't yet convinced myself the current code is right,
> the changes don't seem to make it any worse.  So consider all four
> 
> Reviewed-by: George Spelvin <linux@horizon.com>

Thanks George!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1247576

* Jason Low <jason.low2@hp.com> wrote:

> While running a database workload on a 16 socket machine, there were
> scalability issues related to itimers. The following link contains a
> more detailed summary of the issues at the application level.
> 
> https://lkml.org/lkml/2015/8/26/737
> 
> Commit 1018016c706f addressed the issue with the thread_group_cputimer
> spinlock taking up a significant portion of total run time.
> This patch series addresses the secondary issue where a lot of time is
> spent trying to acquire the sighand lock. It was found in some cases
> that 200+ threads were simultaneously contending for the same sighand
> lock, reducing throughput by more than 30%.
> 
> With this patch set (along with commit 1018016c706f mentioned above),
> the performance hit of itimers almost completely goes away on the
> 16 socket system.
> 
> Jason Low (4):
>   timer: Optimize fastpath_timer_check()
>   timer: Check thread timers only when there are active thread timers
>   timer: Convert cputimer->running to bool
>   timer: Reduce unnecessary sighand lock contention
> 
>  include/linux/init_task.h      |    3 +-
>  include/linux/sched.h          |    9 ++++--
>  kernel/fork.c                  |    2 +-
>  kernel/time/posix-cpu-timers.c |   63 ++++++++++++++++++++++++++++-----------
>  4 files changed, 54 insertions(+), 23 deletions(-)

Is there some itimers benchmark that can be used to measure the effects of these 
changes?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1248091

On Thu, 2015-10-15 at 10:47 +0200, Ingo Molnar wrote:
> * Jason Low <jason.low2@hp.com> wrote:
> 
> > While running a database workload on a 16 socket machine, there were
> > scalability issues related to itimers. The following link contains a
> > more detailed summary of the issues at the application level.
> > 
> > https://lkml.org/lkml/2015/8/26/737
> > 
> > Commit 1018016c706f addressed the issue with the thread_group_cputimer
> > spinlock taking up a significant portion of total run time.
> > This patch series addresses the secondary issue where a lot of time is
> > spent trying to acquire the sighand lock. It was found in some cases
> > that 200+ threads were simultaneously contending for the same sighand
> > lock, reducing throughput by more than 30%.
> > 
> > With this patch set (along with commit 1018016c706f mentioned above),
> > the performance hit of itimers almost completely goes away on the
> > 16 socket system.
> > 
> > Jason Low (4):
> >   timer: Optimize fastpath_timer_check()
> >   timer: Check thread timers only when there are active thread timers
> >   timer: Convert cputimer->running to bool
> >   timer: Reduce unnecessary sighand lock contention
> > 
> >  include/linux/init_task.h      |    3 +-
> >  include/linux/sched.h          |    9 ++++--
> >  kernel/fork.c                  |    2 +-
> >  kernel/time/posix-cpu-timers.c |   63 ++++++++++++++++++++++++++++-----------
> >  4 files changed, 54 insertions(+), 23 deletions(-)
> 
> Is there some itimers benchmark that can be used to measure the effects of these 
> changes?

Yes, we also wrote a micro benchmark which generates cache misses and
measures the average cost of each cache miss (with itimers enabled). We
used this while writing and testing patches, since it takes a bit longer
to set up and run the database.

Jason

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1248391

* Jason Low <jason.low@hpe.com> wrote:

> > > With this patch set (along with commit 1018016c706f mentioned above),
> > > the performance hit of itimers almost completely goes away on the
> > > 16 socket system.
> > > 
> > > Jason Low (4):
> > >   timer: Optimize fastpath_timer_check()
> > >   timer: Check thread timers only when there are active thread timers
> > >   timer: Convert cputimer->running to bool
> > >   timer: Reduce unnecessary sighand lock contention
> > > 
> > >  include/linux/init_task.h      |    3 +-
> > >  include/linux/sched.h          |    9 ++++--
> > >  kernel/fork.c                  |    2 +-
> > >  kernel/time/posix-cpu-timers.c |   63 ++++++++++++++++++++++++++++-----------
> > >  4 files changed, 54 insertions(+), 23 deletions(-)
> > 
> > Is there some itimers benchmark that can be used to measure the effects of these 
> > changes?
> 
> Yes, we also wrote a micro benchmark which generates cache misses and measures 
> the average cost of each cache miss (with itimers enabled). We used this while 
> writing and testing patches, since it takes a bit longer to set up and run the 
> database.

Mind posting it, so that people can stick it into a new 'perf bench timer' 
subcommand, and/or reproduce your results with it?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1249021

On Fri, 2015-10-16 at 09:12 +0200, Ingo Molnar wrote:
> * Jason Low <jason.low@hpe.com> wrote:
> 
> > > > With this patch set (along with commit 1018016c706f mentioned above),
> > > > the performance hit of itimers almost completely goes away on the
> > > > 16 socket system.
> > > > 
> > > > Jason Low (4):
> > > >   timer: Optimize fastpath_timer_check()
> > > >   timer: Check thread timers only when there are active thread timers
> > > >   timer: Convert cputimer->running to bool
> > > >   timer: Reduce unnecessary sighand lock contention
> > > > 
> > > >  include/linux/init_task.h      |    3 +-
> > > >  include/linux/sched.h          |    9 ++++--
> > > >  kernel/fork.c                  |    2 +-
> > > >  kernel/time/posix-cpu-timers.c |   63 ++++++++++++++++++++++++++++-----------
> > > >  4 files changed, 54 insertions(+), 23 deletions(-)
> > > 
> > > Is there some itimers benchmark that can be used to measure the effects of these 
> > > changes?
> > 
> > Yes, we also wrote a micro benchmark which generates cache misses and measures 
> > the average cost of each cache miss (with itimers enabled). We used this while 
> > writing and testing patches, since it takes a bit longer to set up and run the 
> > database.
> 
> Mind posting it, so that people can stick it into a new 'perf bench timer' 
> subcommand, and/or reproduce your results with it?

Yes, sure. At the moment, this micro benchmark is written in C++ and
integrated with the database code. We can look into rewriting it into a
more general program so that it can be included in perf.

Thanks,
Jason

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1249039

Removing dependency to the database code is trivial. It's just 100 lines 
that launch lots of threads and do NUMA-aware memory accesses so that 
remote NUMA access cost does not affect the benchmark.

It's just a bit tedious to convert the C++11 code into C/pthread.
C++11 really spoiled me.
Still, not much work. Let me know where to post the code.

On 10/16/2015 10:34 AM, Jason Low wrote:
>> Mind posting it, so that people can stick it into a new 'perf bench timer'
>> subcommand, and/or reproduce your results with it?
>
> Yes, sure. At the moment, this micro benchmark is written in C++ and
> integrated with the database code. We can look into rewriting it into a
> more general program so that it can be included in perf.
>
> Thanks,
> Jason
>

-- 
Hideaki Kimura
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web