Re: [PATCH 2/3] timer: Check thread timers only when there are active thread timers

Re: [PATCH 2/3] timer: Check thread timers only when there are active thread timers

[George Spelvin]

-	check_thread_timers(tsk, &firing);
+	if (!task_cputime_zero(&tsk->cputime_expires))
+		check_thread_timers(tsk, &firing);

Sincere question; I'm not certain myself: would it make more sense to put
this shortcut into check_thread_timers()?

It seems more like an optimization of that function than something the
caller needs to know about.

Re: [PATCH 2/3] timer: Check thread timers only when there are active thread timers

[Jason Low]

On Wed, 2015-08-26 at 13:04 -0400, George Spelvin wrote:
> -	check_thread_timers(tsk, &firing);
> +	if (!task_cputime_zero(&tsk->cputime_expires))
> +		check_thread_timers(tsk, &firing);
> 
> Sincere question; I'm not certain myself: would it make more sense to put
> this shortcut into check_thread_timers()?
> 
> It seems more like an optimization of that function than something the
> caller needs to know about.

Yes, I also thought it might be better if we add something like:

	if (task_cputime_zero(&tsk->cputime_expires)
		return;

in check_thread_timers(). The reason I made it this way though is
because in the next few lines, we do a similar check before calling
check_process_timers(), and I wanted to keep things consistent.

However, perhaps we can consider also moving that
tsk->signal->cputimer.running check into check_process_timers() too.

Thanks for the suggestions.

[PATCH 0/3] timer: Improve itimers scalability

[Jason Low]

When running a database workload on a 16 socket machine, there were
scalability issues related to itimers.

Commit 1018016c706f addressed the issue with the thread_group_cputimer
spinlock taking up a significant portion of total run time.

This patch series address the other 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%.

Jason Low (3):
  timer: Optimize fastpath_timer_check()
  timer: Check thread timers only when there are active thread timers
  timer: Reduce unnecessary sighand lock contention

 include/linux/init_task.h      |    1 +
 include/linux/sched.h          |    3 ++
 kernel/time/posix-cpu-timers.c |   44 +++++++++++++++++++++++++++------------
 3 files changed, 34 insertions(+), 14 deletions(-)

-- 
1.7.2.5

[PATCH 2/3] timer: Check thread timers only when there are active thread timers

[Jason Low]

The fastpath_timer_check() contains logic to check for if any timers
are set by checking if !task_cputime_zero(). Similarly, we can do this
before calling check_thread_timers(). In the case where there
are only process-wide timers, this will skip all the computations for
the per-thread timers when there are no per-thread timers.

Signed-off-by: Jason Low <jason.low2@hp.com>
---
 kernel/time/posix-cpu-timers.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 02596ff..535bef5 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -1168,11 +1168,13 @@ void run_posix_cpu_timers(struct task_struct *tsk)
 	if (!lock_task_sighand(tsk, &flags))
 		return;
 	/*
-	 * Here we take off tsk->signal->cpu_timers[N] and
-	 * tsk->cpu_timers[N] all the timers that are firing, and
-	 * put them on the firing list.
+	 * If there are active per-thread timers, take off
+	 * tsk->signal->cpu_timers[N] and tsk->cpu_timers[N] all the
+	 * timers that are firing, and put them on the firing list.
 	 */
-	check_thread_timers(tsk, &firing);
+	if (!task_cputime_zero(&tsk->cputime_expires))
+		check_thread_timers(tsk, &firing);
+
 	/*
 	 * If there are any active process wide timers (POSIX 1.b, itimers,
 	 * RLIMIT_CPU) cputimer must be running.
-- 
1.7.2.5