mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jason Low <jason.low2@hp.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Oleg Nesterov <oleg@redhat.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Steven Rostedt <rostedt@goodmis.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Terry Rudd <terry.rudd@hp.com>, Rik van Riel <riel@redhat.com>,
	Scott J Norton <scott.norton@hp.com>,
	Jason Low <jason.low2@hp.com>
Subject: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention
Date: Tue, 25 Aug 2015 20:17:48 -0700	[thread overview]
Message-ID: <1440559068-29680-4-git-send-email-jason.low2@hp.com> (raw)
In-Reply-To: <1440559068-29680-1-git-send-email-jason.low2@hp.com>

It was found while running a database workload on large systems that
significant time was spent trying to acquire the sighand lock.

The issue was that whenever an itimer expired, many threads ended up
simultaneously trying to send the signal. Most of the time, nothing
happened after acquiring the sighand lock because another thread
had already sent the signal and updated the "next expire" time. The
fastpath_timer_check() didn't help much since the "next expire" time
was updated later.
 
This patch addresses this by having the thread_group_cputimer structure
maintain a boolean to signify when a thread in the group is already
checking for process wide timers, and adds extra logic in the fastpath
to check the boolean.

Signed-off-by: Jason Low <jason.low2@hp.com>
---
 include/linux/init_task.h      |    1 +
 include/linux/sched.h          |    3 +++
 kernel/time/posix-cpu-timers.c |   19 +++++++++++++++++--
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index d0b380e..3350c77 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -53,6 +53,7 @@ extern struct fs_struct init_fs;
 	.cputimer	= { 						\
 		.cputime_atomic	= INIT_CPUTIME_ATOMIC,			\
 		.running	= 0,					\
+		.checking_timer	= 0,					\
 	},								\
 	INIT_PREV_CPUTIME(sig)						\
 	.cred_guard_mutex =						\
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 119823d..a6c8334 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -619,6 +619,8 @@ struct task_cputime_atomic {
  * @cputime_atomic:	atomic thread group interval timers.
  * @running:		non-zero when there are timers running and
  * 			@cputime receives updates.
+ * @checking_timer:	non-zero when a thread is in the process of
+ *			checking for thread group timers.
  *
  * This structure contains the version of task_cputime, above, that is
  * used for thread group CPU timer calculations.
@@ -626,6 +628,7 @@ struct task_cputime_atomic {
 struct thread_group_cputimer {
 	struct task_cputime_atomic cputime_atomic;
 	int running;
+	int checking_timer;
 };
 
 #include <linux/rwsem.h>
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 535bef5..f3ddf0e 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -962,6 +962,14 @@ static void check_process_timers(struct task_struct *tsk,
 	unsigned long soft;
 
 	/*
+	 * Signify that a thread is checking for process timers.
+	 * The checking_timer field is only modified in this function,
+	 * which is called with the sighand lock held. Thus, we can
+	 * just use WRITE_ONCE() without any further locking.
+	 */
+	WRITE_ONCE(sig->cputimer.checking_timer, 1);
+
+	/*
 	 * Collect the current process totals.
 	 */
 	thread_group_cputimer(tsk, &cputime);
@@ -1015,6 +1023,8 @@ static void check_process_timers(struct task_struct *tsk,
 	sig->cputime_expires.sched_exp = sched_expires;
 	if (task_cputime_zero(&sig->cputime_expires))
 		stop_process_timers(sig);
+
+	WRITE_ONCE(sig->cputimer.checking_timer, 0);
 }
 
 /*
@@ -1132,8 +1142,13 @@ static inline int fastpath_timer_check(struct task_struct *tsk)
 	}
 
 	sig = tsk->signal;
-	/* Check if cputimer is running. This is accessed without locking. */
-	if (READ_ONCE(sig->cputimer.running)) {
+	/*
+	 * Check if thread group timers expired if the cputimer is running
+	 * and that no other thread in the group is already checking for
+	 * thread group cputimers.
+	 */
+	if (READ_ONCE(sig->cputimer.running) &&
+	    !READ_ONCE(sig->cputimer.checking_timer)) {
 		struct task_cputime group_sample;
 
 		sample_cputime_atomic(&group_sample, &sig->cputimer.cputime_atomic);
-- 
1.7.2.5


  parent reply	other threads:[~2015-08-26  3:18 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-26  3:17 [PATCH 0/3] timer: Improve itimers scalability Jason Low
2015-08-26  3:17 ` [PATCH 1/3] timer: Optimize fastpath_timer_check() Jason Low
2015-08-26 21:57   ` Frederic Weisbecker
2015-08-31 15:15   ` Davidlohr Bueso
2015-08-31 19:40     ` Jason Low
2015-08-26  3:17 ` [PATCH 2/3] timer: Check thread timers only when there are active thread timers Jason Low
2015-08-26  3:17 ` Jason Low [this message]
2015-08-26 17:53   ` [PATCH 3/3] timer: Reduce unnecessary sighand lock contention Linus Torvalds
2015-08-26 22:31     ` Frederic Weisbecker
2015-08-26 22:57       ` Jason Low
2015-08-26 22:56   ` Frederic Weisbecker
2015-08-26 23:32     ` Jason Low
2015-08-27  4:52       ` Jason Low
2015-08-27 12:53       ` Frederic Weisbecker
2015-08-27 20:29         ` Jason Low
2015-08-27 21:12           ` Frederic Weisbecker
2015-08-26  3:27 ` [PATCH 0/3] timer: Improve itimers scalability Andrew Morton
2015-08-26 16:33   ` Jason Low
2015-08-26 17:08     ` Oleg Nesterov
2015-08-26 22:07       ` Jason Low
2015-08-26 22:53         ` Hideaki Kimura
2015-08-26 23:13           ` Frederic Weisbecker
2015-08-26 23:45             ` Hideaki Kimura
2015-08-27 13:18               ` Frederic Weisbecker
2015-08-27 14:47                 ` Steven Rostedt
2015-08-27 15:09                   ` Thomas Gleixner
2015-08-27 15:17                     ` Frederic Weisbecker
2015-08-26 19:33 [PATCH 3/3] timer: Reduce unnecessary sighand lock contention George Spelvin
2015-08-26 23:44 ` Jason Low
2015-08-27  1:28   ` George Spelvin
2015-08-27 21:55     ` Jason Low
2015-08-27 22:43       ` George Spelvin
2015-08-28  4:32         ` Jason Low
2015-08-26 21:05 George Spelvin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1440559068-29680-4-git-send-email-jason.low2@hp.com \
    --to=jason.low2@hp.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave@stgolabs.net \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=scott.norton@hp.com \
    --cc=terry.rudd@hp.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®