Re: [RFC PATCH 13/13] m68k: mvme16x: Convert to clocksource APIy

From: Thomas Gleixner <tglx@linutronix.de>
To: Finn Thain <fthain@telegraphics.com.au>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Stephen N Chivers <schivers@csc.com.au>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	John Stultz <john.stultz@linaro.org>,
	linux-m68k@lists.linux-m68k.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 13/13] m68k: mvme16x: Convert to clocksource APIy
Date: Sun, 18 Nov 2018 18:12:46 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.21.1811181759330.9459@nanos.tec.linutronix.de> (raw)
In-Reply-To: <alpine.LNX.2.21.1811140925270.371@nippy.intranet>

Finn,

On Wed, 14 Nov 2018, Finn Thain wrote:
> On Tue, 13 Nov 2018, Thomas Gleixner wrote:
> > Urgh. Then you have more serious trouble. If the interrupting handler 
> > calls any of the time accessor functions then you can actually live lock 
> > when the interrupt happens in the middle of the write locked section of 
> > the core timekeeping update.
> 
> Does this apply to arch_gettimeoffset also? If so, that would mean another 
> bug fix patch for -stable...

arch_gettimeoffset() can be called from any context. The only interrupt
which can affect it is the timer interrupt. As m68k are all UP machines
this is not a problem because:

  ktime_get()
     do {
        seq = read_seqcount_begin(tk.seq);
	nsec = .... + arch_gettimeoffset();
     } while (seqcount_retry(tk.seq);

and the timer interrupt does:

  arch_timeoffset += value;
  ...
    xtime_update()
      ...
        write_seqcount_begin(tk.seq);
        ....
        write_seqcount_end(tk.seq);

So where ever the timer interrupt hits inside of ktime_get() or any other
accessor the seqcount will make it retry, so the result is always
consistent.

For SMP that would be a different story.

> > So you really want to disable interrupts across the whole timer 
> > interrupt function or make sure that the timer interrupt is the highest 
> > priority one on the system.
> > 
> 
> The timer interrupt priority isn't always what one would hope. 
> 
> But I can certainly disable interrupts for timer_interrupt() execution 
> (that is, xtime_update() etc.)

Oh yes, you should.

Thanks,

	tglx