Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1507271

Re: [GIT pull] timer updates for 4.9

From Thomas Gleixner <tglx@linutronix.de>
Newsgroups linux.kernel
Subject Re: [GIT pull] timer updates for 4.9
Date 2016-10-24 17:00 +0200
Message-ID <svOTf-vq-19@gated-at.bofh.it> (permalink)
References <sv3hD-3en-11@gated-at.bofh.it> <svzKy-785-51@gated-at.bofh.it> <svAng-7PA-5@gated-at.bofh.it> <svK3f-5Kp-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, 24 Oct 2016, Thomas Gleixner wrote:
> On Sun, 23 Oct 2016, Linus Torvalds wrote:
> > So I found what looks like a bug in lock_timer_base() wrt migration.
> > 
> > This code:
> > 
> >         for (;;) {
> >                 struct timer_base *base;
> >                 u32 tf = timer->flags;
> > 
> >                 if (!(tf & TIMER_MIGRATING)) {
> >                         base = get_timer_base(tf);
> >                         spin_lock_irqsave(&base->lock, *flags);
> >                         if (timer->flags == tf)
> >                                 return base;
> >                         spin_unlock_irqrestore(&base->lock, *flags);
> >                 }
> >                 cpu_relax();
> >         }
> > 
> > looks subtly buggy. I think that load of "tf" needs a READ_ONCE() to
> > make sure that gcc doesn't simply reload the valid of "timer->flags"
> > at random points.
> 
> You are right, that needs a READ_ONCE(). Stupid me.
>  
> > Yes, the spin_lock_irqsave() is a barrier, but that's the only one.
> > Afaik, gcc could decide that "I need to spill tf, so I'll just reload
> > it" after looking up get_timer_base().
> >
> > And no, I don't think this is the cause of my problem, but I suspect
> > that something _like_ fragility in lock_timer_base() could cause this.
> 
> It might explain it, when this really ends up with the wrong base.

Can you please check in the disassembly whether gcc really reloads
timer->flags? Mine does not...

Another thing you might try is to enable debugobjects. As this happens only
at shutdown time the problem might be caused by something else which
wreckages timers in some subtle way.

Thanks,

	tglx

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[GIT pull] timer updates for 4.9 Thomas Gleixner <tglx@linutronix.de> - 2016-10-22 14:10 +0200
  Re: [GIT pull] timer updates for 4.9 Linus Torvalds <torvalds@linux-foundation.org> - 2016-10-24 00:50 +0200
    Re: [GIT pull] timer updates for 4.9 Linus Torvalds <torvalds@linux-foundation.org> - 2016-10-24 01:30 +0200
      Re: [GIT pull] timer updates for 4.9 Thomas Gleixner <tglx@linutronix.de> - 2016-10-24 11:50 +0200
        Re: [GIT pull] timer updates for 4.9 Thomas Gleixner <tglx@linutronix.de> - 2016-10-24 17:00 +0200
          Re: [GIT pull] timer updates for 4.9 Thomas Gleixner <tglx@linutronix.de> - 2016-10-24 17:20 +0200
            [tip:timers/urgent] timers: Plug locking race vs. timer migration tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2016-10-25 17:00 +0200
            [tip:timers/urgent] timers: Lock base for same bucket optimization tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2016-10-25 17:00 +0200
          Re: [GIT pull] timer updates for 4.9 Linus Torvalds <torvalds@linux-foundation.org> - 2016-10-24 19:20 +0200
            Re: [GIT pull] timer updates for 4.9 Thomas Gleixner <tglx@linutronix.de> - 2016-10-24 21:20 +0200
              Re: [GIT pull] timer updates for 4.9 Thomas Gleixner <tglx@linutronix.de> - 2016-10-24 23:40 +0200

csiph-web