Re: [PATCH 3/3] timer: Reduce unnecessary sighand lock contention

Jason Low
2015-08-27T04:53:04Z

On Wed, 2015-08-26 at 16:32 -0700, Jason Low wrote:

> Perhaps to be safer, we use something like load_acquire() and
> store_release() for accessing both the ->running and ->checking_timer
> fields?

Regarding using barriers, one option could be to pair them between
sig->cputime_expires and the sig->cputimer.checking_timer accesses.

fastpath_timer_check()
{
...

if (READ_ONCE(sig->cputimer.running))
struct task_cputime group_sample;

sample_cputime_atomic(&group_sample, &sig->cputimer.cputime_atomic);

if (task_cputime_expired(&group_sample, &sig->cputime_expires)) {
/*
* Comments
*/
mb();

if (!READ_ONCE(sig->cputimer.checking_timer))
return 1;
}
}
}

check_process_timers()
{
...

WRITE_ONCE(sig->cputimer.checking_timer, 0);

/*
* Comments
*/
mb();

sig->cputime_expires.prof_exp = expires_to_cputime(prof_expires);
sig->cputime_expires.virt_exp = expires_to_cputime(virt_expires);
sig->cputime_expires.sched_exp = sched_expires;

...
}

By the time the cputime_expires fields get updated at the end of
check_process_timers(), other threads in the fastpath_timer_check()
should observe the value 0 for READ_ONCE(sig->cputimer.checking_timer).

In the case where threads in the fastpath don't observe the
WRITE_ONCE(checking_timer, 1) early enough, that's fine, since it will
just (unnecessarily) go through the slowpath which is what we also do in
the current code.