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

[Jason Low <jason.low2@hp.com>]

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.