LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: Finn Thain <fthain@telegraphics.com.au>
Cc: Christoph Hellwig <hch@infradead.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Stephen N Chivers <schivers@csc.com.au>,
	Thomas Gleixner <tglx@linutronix.de>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	John Stultz <john.stultz@linaro.org>,
	linux-m68k@lists.linux-m68k.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH 01/13] arm: Fix mutual exclusion in arch_gettimeoffset
Date: Wed, 14 Nov 2018 14:16:32 +0000
Message-ID: <20181114141632.GT30658@n2100.armlinux.org.uk> (raw)
In-Reply-To: <alpine.LNX.2.21.1811141142490.8@nippy.intranet>

On Wed, Nov 14, 2018 at 02:17:09PM +1100, Finn Thain wrote:
> On Tue, 13 Nov 2018, Russell King - ARM Linux wrote:
> 
> > 
> > A clocksource provides a cycle counter that monotonically changes and 
> > does not wrap between clockevent events.
> > 
> > A clock event is responsible for providing events to the system when 
> > some work is needing to be done, limited by the wrap interval of the 
> > clocksource.
> > 
> > Each time the clock event triggers an interrupt, the clocksource is
> > read to determine how much time has passed, using:
> > 
> > 	count = (new_value - old_value) & available_bits
> > 	nanosecs = count * scale >> shift;
> > 
> > If you try to combine the clocksource and clockevent because you only
> > have a single counter, and the counter has the behaviour of:
> > - counting down towards zero
> > - when reaching zero, triggers an interrupt, and reloads with N
> > 
> > then this provides your clockevent, but you can't use this as a clock
> > source, because each time you receive an interrupt and try to read the
> > counter value, it will be approximately the same value.  This means
> > that the above calculation fails to register the correct number of
> > nanoseconds passing.  Hence, this does not work.
> > 
> > Also note where I said above that the clock event device must be able
> > to provide an interrupt _before_ the clocksource wraps - clearly with
> > such a timer, that is utterly impossible.
> > 
> > The simple solution is to not use such a counter as the clocksource,
> > which means you fall back to using the jiffies clocksource, and your
> > timekeeping has jiffy resolution - so 10ms, or possibly 1ms if you
> > want a 1kHz timer interval.  For most applications, that's simply way
> > to coarse, as was realised in the very early days of Linux.
> > 
> > If only there was a way to interpolate between timer interrupts...
> > which is exactly what arch_gettimeoffset() does, and is a historical
> > reminant of the pre-clocksource/clockevent days of the kernel - but
> > it is the only way to get better resolution from this sort of setup.
> > 
> 
> Both of the platforms in question (RPC and EBSA110) have not 
> defined(CONFIG_GENERIC_CLOCKEVENTS) and have not defined any struct 
> clock_event_device, AFAICT.

Correct, because they don't use clockevents.  This is pre-clocksource,
pre-clockevent code, it uses the old timekeeping infrastructure,
which is xtime_update() and providing a gettimeoffset implementation.

> So, even assuming that you're right about the limitations of single-timer 
> platforms in general, removal of arch_gettimeoffset wouldn't require the 
> removal of any platforms, AFAICT.

I haven't proposed removing platforms.

I'm just objecting to the idea of removing arch_gettimeoffset(),
thereby causing a regression by changing the resolution of
gettimeofday() without any sign of equivalent functionality.

However, I now see (having searched mailing lists) what you are
trying to do - you have _not_ copied me or the mailing lists I'm
on with your cover message, so I'm *totally* lacking in the context
of your patch series, particularly where you are converting m68k
to use clocksources without needing the gettimeoffset() stuff.

You have failed to explain that in this thread - probably assuming
that I've read your cover message.  I haven't until now, because
you never sent it to me or the linux-arm-kernel mailing list.

I have found this thread _very_ frustrating, and frankly a waste of
my time discussing the finer points because of this lack of context.
Please ensure that if you're going to be sending a patch series,
that the cover message at least finds its way to the intended
audience of your patches, so that everyone has the context they
need when looking at (eg) the single patch they may receive.

Alternatively, if someone raises a problem with the patch, and you
_know_ you haven't done that, then please consider informing them
where they can get more context, eg, by providing a link to your
archived cover message.  It would help avoid misunderstandings.

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

  reply index

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-12  4:12 [RFC PATCH 00/13] m68k: Drop arch_gettimeoffset and adopt clocksource API Finn Thain
2018-11-12  4:12 ` [RFC PATCH 08/13] m68k: atari: Convert to " Finn Thain
2018-11-12  4:12 ` [RFC PATCH 04/13] m68k: mac: Clean up unused timer definitions Finn Thain
2018-11-12  4:12 ` [RFC PATCH 10/13] m68k: hp300: Convert to clocksource API Finn Thain
2018-11-12  4:12 ` [RFC PATCH 11/13] m68k: mac: " Finn Thain
2018-11-12  4:12 ` [RFC PATCH 01/13] arm: Fix mutual exclusion in arch_gettimeoffset Finn Thain
2018-11-12  8:34   ` Christoph Hellwig
2018-11-13  3:39     ` Finn Thain
2018-11-13  9:20       ` Russell King - ARM Linux
2018-11-13 21:55         ` Finn Thain
2018-11-13 23:43           ` Russell King - ARM Linux
2018-11-14  1:35             ` Michael Schmitz
2018-11-14  7:58               ` Russell King - ARM Linux
2018-11-15  1:34                 ` Michael Schmitz
2018-11-14  3:17             ` Finn Thain
2018-11-14 14:16               ` Russell King - ARM Linux [this message]
2018-11-14 14:58                 ` Geert Uytterhoeven
2018-11-14 18:11                   ` Russell King - ARM Linux
2018-11-15  4:12                 ` Finn Thain
2018-11-16 17:47                   ` Russell King - ARM Linux
2018-11-16 22:49                     ` Finn Thain
2018-11-17  3:00                       ` Michael Schmitz
2018-11-14 12:05   ` Russell King - ARM Linux
2018-11-12  4:12 ` [RFC PATCH 02/13] m68k: " Finn Thain
2018-11-12  4:12 ` [RFC PATCH 07/13] m68k: amiga: Convert to clocksource API Finn Thain
2018-11-12  9:02   ` Geert Uytterhoeven
2018-11-12  9:21     ` Finn Thain
2018-11-12  4:12 ` [RFC PATCH 09/13] m68k: bvme6000: " Finn Thain
2018-11-12  4:12 ` [RFC PATCH 06/13] m68k: Drop ARCH_USES_GETTIMEOFFSET Finn Thain
2018-11-12  9:00   ` Geert Uytterhoeven
2018-11-12  9:06     ` Finn Thain
2018-11-13  2:29       ` Michael Schmitz
2018-11-13  3:14         ` Finn Thain
2018-11-13  4:50           ` Michael Schmitz
2018-11-13  6:15             ` Finn Thain
2018-11-13  8:24               ` Michael Schmitz
2018-11-13 22:11                 ` Finn Thain
2018-11-14  1:08                   ` Michael Schmitz
2018-11-14  2:58                     ` Michael Schmitz
2018-11-14 23:54                       ` Michael Schmitz
2018-11-15  4:37                         ` Finn Thain
2018-11-15  6:35                         ` Michael Schmitz
2018-11-16  0:04                           ` Finn Thain
2018-11-12  4:12 ` [RFC PATCH 13/13] m68k: mvme16x: Convert to clocksource API Finn Thain
2018-11-12 18:37   ` Thomas Gleixner
2018-11-13  0:11     ` Finn Thain
2018-11-13 22:04       ` Finn Thain
2018-11-13 22:10         ` [RFC PATCH 13/13] m68k: mvme16x: Convert to clocksource APIy Thomas Gleixner
2018-11-13 22:33           ` Finn Thain
2018-11-18 17:12             ` Thomas Gleixner
2018-11-12  4:12 ` [RFC PATCH 03/13] m68k: mac: Fix VIA timer counter accesses Finn Thain
2018-11-12  4:12 ` [RFC PATCH 12/13] m68k: mvme147: Convert to clocksource API Finn Thain
2018-11-12  4:12 ` [RFC PATCH 05/13] m68k: apollo, q40, sun3, sun3x: Remove arch_gettimeoffset implementations Finn Thain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181114141632.GT30658@n2100.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=arnd@arndb.de \
    --cc=daniel.lezcano@linaro.org \
    --cc=fthain@telegraphics.com.au \
    --cc=geert@linux-m68k.org \
    --cc=hch@infradead.org \
    --cc=john.stultz@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=schivers@csc.com.au \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lore.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git