Re: [Question]: try to fix contention between expire_timers and try_to_del_timer_sync

From	Will Deacon <will.deacon@arm.com>
Newsgroups	linux.kernel
Subject	Re: [Question]: try to fix contention between expire_timers and try_to_del_timer_sync
Date	2017-07-27 17:20 +0200
Message-ID	<u7SJY-7Iw-17@gated-at.bofh.it> (permalink)
References	<u7vkm-1ob-21@gated-at.bofh.it> <u7vkm-1ob-19@gated-at.bofh.it> <u7FWq-82k-11@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Thu, Jul 27, 2017 at 09:29:20AM +0800, qiaozhou wrote:
> On 2017年07月26日 22:16, Thomas Gleixner wrote:
> >--- a/kernel/time/timer.c
> >+++ b/kernel/time/timer.c
> >@@ -1301,10 +1301,12 @@ static void expire_timers(struct timer_b
> >  		if (timer->flags & TIMER_IRQSAFE) {
> >  			raw_spin_unlock(&base->lock);
> >  			call_timer_fn(timer, fn, data);
> >+			base->running_timer = NULL;
> >  			raw_spin_lock(&base->lock);
> >  		} else {
> >  			raw_spin_unlock_irq(&base->lock);
> >  			call_timer_fn(timer, fn, data);
> >+			base->running_timer = NULL;
> >  			raw_spin_lock_irq(&base->lock);
> >  		}
> >  	}
> It should work for this particular issue and I'll test it. Previously I
> thought it was unsafe to touch base->running_timer without holding lock.

I think it works out in practice because base->lock and base->running_timer
share a cacheline, so end up being ordered correctly. We should probably be
using READ_ONCE/WRITE_ONCE for accessing the running_time field though.

One thing I don't get though, is why try_to_del_timer_sync needs to check
base->running_timer at all. Given that it holds the base->lock, can't it
be the person that sets it to NULL?

Will

Thread

Re: [Question]: try to fix contention between expire_timers and  try_to_del_timer_sync Thomas Gleixner <tglx@linutronix.de> - 2017-07-26 16:20 +0200
  Re: [Question]: try to fix contention between expire_timers and  try_to_del_timer_sync qiaozhou <qiaozhou@asrmicro.com> - 2017-07-27 03:40 +0200
    Re: [Question]: try to fix contention between expire_timers and  try_to_del_timer_sync Thomas Gleixner <tglx@linutronix.de> - 2017-07-27 17:20 +0200
    Re: [Question]: try to fix contention between expire_timers and  try_to_del_timer_sync Will Deacon <will.deacon@arm.com> - 2017-07-27 17:20 +0200
    Re: [Question]: try to fix contention between expire_timers and  try_to_del_timer_sync Vikram Mulukutla <markivx@codeaurora.org> - 2017-07-28 03:20 +0200
      Re: [Question]: try to fix contention between expire_timers and  try_to_del_timer_sync Will Deacon <will.deacon@arm.com> - 2017-07-28 11:30 +0200
        Re: [Question]: try to fix contention between expire_timers and  try_to_del_timer_sync Vikram Mulukutla <markivx@codeaurora.org> - 2017-07-28 21:10 +0200
          Re: [Question]: try to fix contention between expire_timers and  try_to_del_timer_sync qiaozhou <qiaozhou@asrmicro.com> - 2017-07-31 13:30 +0200
      Re: [Question]: try to fix contention between expire_timers and  try_to_del_timer_sync Peter Zijlstra <peterz@infradead.org> - 2017-07-28 11:30 +0200
        Re: [Question]: try to fix contention between expire_timers and  try_to_del_timer_sync Vikram Mulukutla <markivx@codeaurora.org> - 2017-07-28 21:20 +0200

csiph-web