All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Jack Winch <sunt.un.morcov@gmail.com>, Arnd Bergmann <arnd@arndb.de>
Cc: "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>
Subject: Re: Suggestion - Configurable Source Clock Type for Line Event Timestamping
Date: Tue, 13 Oct 2020 10:42:59 +0200	[thread overview]
Message-ID: <CACRpkda4DF-XAi5XpJNLU_vjD9Zrjs6PkGpz5BW1E44W67SWvg@mail.gmail.com> (raw)
In-Reply-To: <CAFhCfDa_FNNC7ushPApRguj3Omik27wRjb3Eh1-_4a1js63FVw@mail.gmail.com>

On Sun, Oct 11, 2020 at 5:11 PM Jack Winch <sunt.un.morcov@gmail.com> wrote:

> I recently noticed that in Linux 5.7, gpiolib was changed such that
> line events are now timestamped using the system 'monotonic' clock
> rather than the system realtime clock.  The rationale for this change
> appears to be due to the major use-case of the line event timestamp
> data in relation to the nature of the system realtime clock (which can
> jump backwards or forwards in time due to adjustments by third-parties
> - e.g., NTP or PTP clients, etc).

The background is in the commit:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/gpiolib.c?id=f8850206e160bfe35de9ca2e726ab6d6b8cb77dd

Also study the solution in IIO that started the discussion about
all this:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=bc2b7dab629a51e8beb5fda4222c62a23b729f26

> I know there are users of the line event timestamp who actively
> rely on that value being obtained from the system realtime clock.

(then in follow-up mail)
> For context, these wall clock time sensitive users are running on
> systems which are PTPv2 clients, with their system realtime clock
> synchronised to that of a local PTP Grand Master clock.  In the past,
> I have used the TTL Pulse Per Second (PPS) output of the Grand Master
> to evaluate methods of timestamping line events with wall clock time
> and it was the kernel timestamping which was most suitable for our
> application.

As Arnd stated in the thread from 2018:
"most of these clocks make no sense at all for a random user
space interface, mainly because I wouldn't trust user space
programmers to make an informed decision which of those
seven to use."

So I suspect you actually managed to make a good argument
for using the realtime clock, we didn't see that coming. :)

But why can't userspace (whether your application or libgpiod)
just call the system clock_gettime() function at the arrival of a
lineevent in that case, and this will be quick due to using vDSO
on most arch:es so no user-to-kernelspace switch will be
required?

clock_gettime() can get you any of the things IIO can get
you.

If you really need the timestamp to be as close as possible
to the actual event then I see why you want the kernel to
make the timestamp in the hard IRQ handler already, but
I just want to confirm that you really really need this.

> My suggestion (which I would be happy to implement myself) is to allow
> users to select the clock to be used for line event timestamping on a
> per line handle basis.

I suppose that makes more sense than the "global switch" in sysfs
for the whole device that IIO uses. At least it is a clear sign that the
user wants this specific type of timestamp.

> 1. Increase in processing overhead and latency of timestamp
> acquisition on line event interrupts.  Implementing the proposed
> change requires a function call to be made to the appropriate ktime
> accessor function, based on what the user has configured as the
> timestamp clock source.  In kernel versions from 5.7 to current, a
> call is made to the ktime_get_ns() function which is most likely
> inlined by the compiler.  This change will result in an actual jump
> having to be made, which will have processor and memory access
> overhead (potential I$ and D$ misses).  Then there is of course the
> overhead of resolving which function to call - either a switch
> statement or call by function pointer (probably the latter option).

Given the overall latency of the kernelspace to userspace
switch and the whole kernel executing around us this is not of
any big concern I'd say, though I will stand corrected in the
face of real-world usecase.

Yours,
Linus Walleij

  parent reply	other threads:[~2020-10-13  8:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-11 14:15 Suggestion - Configurable Source Clock Type for Line Event Timestamping Jack Winch
2020-10-12  5:06 ` Kent Gibson
2020-10-12  7:23   ` Jack Winch
2020-10-12  9:14     ` Kent Gibson
2020-10-12 10:21 ` Bartosz Golaszewski
2020-10-12 10:05   ` Jack Winch
2020-10-12 13:39     ` Bartosz Golaszewski
2020-10-12 14:21       ` Kent Gibson
2020-10-12 14:25         ` Bartosz Golaszewski
2020-10-13  8:42 ` Linus Walleij [this message]
2020-10-14 11:07   ` Jack Winch

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=CACRpkda4DF-XAi5XpJNLU_vjD9Zrjs6PkGpz5BW1E44W67SWvg@mail.gmail.com \
    --to=linus.walleij@linaro.org \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=arnd@arndb.de \
    --cc=linux-gpio@vger.kernel.org \
    --cc=sunt.un.morcov@gmail.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.