Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1214246
| From | Jason Low <jason.low2@hp.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention |
| Date | 2015-08-27 01:40 +0200 |
| Message-ID | <q1Ssr-3Ko-23@gated-at.bofh.it> (permalink) |
| References | <q1zpL-1JJ-1@gated-at.bofh.it> <q1zpM-1JJ-11@gated-at.bofh.it> <q1RPI-2Mm-21@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Thu, 2015-08-27 at 00:56 +0200, Frederic Weisbecker wrote:
> On Tue, Aug 25, 2015 at 08:17:48PM -0700, Jason Low wrote:
> > 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;
>
> How about a flag in the "running" field instead?
>
> 1) Space in signal_struct is not as important as in task_strut but it
> still matters.
George Spelvin suggested that we convert them to booleans which would
make them take up 2 bytes.
> 2) We already read the "running" field locklessly. Adding a new field like
> checking_timer gets even more complicated. Ideally there should be at
> least a paired memory barrier between both. Let's just simplify that
> with a single field.
hmmm, so having 1 "flag" where we access bits for the "running" and
"checking_timer"?
> Now concerning the solution for your problem, I'm a bit uncomfortable with
> lockless magics like this. When the thread sets checking_timer to 1, there is
> no guarantee that the other threads in the process will see it "fast" enough
> to avoid the slow path checks. Then there is also the risk that the threads
> don't see "fast" enough that checking_timers has toggled to 0 and as a result
> a timer may expire late. Now the lockless access of "running" already induces
> such race. So if it really solves issues in practice, why not.
Perhaps to be safer, we use something like load_acquire() and
store_release() for accessing both the ->running and ->checking_timer
fields?
--
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/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 0/3] timer: Improve itimers scalability Jason Low <jason.low2@hp.com> - 2015-08-26 05:20 +0200
[PATCH 2/3] timer: Check thread timers only when there are active thread timers Jason Low <jason.low2@hp.com> - 2015-08-26 05:20 +0200
[PATCH 1/3] timer: Optimize fastpath_timer_check() Jason Low <jason.low2@hp.com> - 2015-08-26 05:20 +0200
Re: [PATCH 1/3] timer: Optimize fastpath_timer_check() Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-27 00:00 +0200
Re: [PATCH 1/3] timer: Optimize fastpath_timer_check() Davidlohr Bueso <dave@stgolabs.net> - 2015-08-31 17:20 +0200
Re: [PATCH 1/3] timer: Optimize fastpath_timer_check() Jason Low <jason.low2@hp.com> - 2015-08-31 21:50 +0200
[PATCH 3/3] timer: Reduce unnecessary sighand lock contention Jason Low <jason.low2@hp.com> - 2015-08-26 05:20 +0200
Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention Linus Torvalds <torvalds@linux-foundation.org> - 2015-08-26 20:00 +0200
Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-27 00:40 +0200
Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention Jason Low <jason.low2@hp.com> - 2015-08-27 01:00 +0200
Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-27 01:00 +0200
Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention Jason Low <jason.low2@hp.com> - 2015-08-27 01:40 +0200
Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention Jason Low <jason.low2@hp.com> - 2015-08-27 07:00 +0200
Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-27 15:00 +0200
Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention Jason Low <jason.low2@hp.com> - 2015-08-27 22:40 +0200
Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-27 23:20 +0200
Re: [PATCH 0/3] timer: Improve itimers scalability Andrew Morton <akpm@linux-foundation.org> - 2015-08-26 05:30 +0200
Re: [PATCH 0/3] timer: Improve itimers scalability Jason Low <jason.low2@hp.com> - 2015-08-26 18:40 +0200
Re: [PATCH 0/3] timer: Improve itimers scalability Oleg Nesterov <oleg@redhat.com> - 2015-08-26 19:20 +0200
Re: [PATCH 0/3] timer: Improve itimers scalability Jason Low <jason.low2@hp.com> - 2015-08-27 00:10 +0200
Re: [PATCH 0/3] timer: Improve itimers scalability Hideaki Kimura <hideaki.kimura@hpe.com> - 2015-08-27 01:20 +0200
Re: [PATCH 0/3] timer: Improve itimers scalability Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-27 01:20 +0200
Re: [PATCH 0/3] timer: Improve itimers scalability Hideaki Kimura <hideaki.kimura@hpe.com> - 2015-08-27 02:00 +0200
Re: [PATCH 0/3] timer: Improve itimers scalability Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-27 15:20 +0200
Re: [PATCH 0/3] timer: Improve itimers scalability Steven Rostedt <rostedt@goodmis.org> - 2015-08-27 16:50 +0200
Re: [PATCH 0/3] timer: Improve itimers scalability Thomas Gleixner <tglx@linutronix.de> - 2015-08-27 17:20 +0200
Re: [PATCH 0/3] timer: Improve itimers scalability Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-27 17:20 +0200
csiph-web