Re: [PATCH 1/4] time: add ktime_get_cycles64() api

John Stultz
2023-09-29T07:07:02Z

On Thu, Sep 28, 2023 at 11:56 PM John Stultz <jstultz@google.com> wrote:
> On Thu, Sep 28, 2023 at 11:35 PM Mahesh Bandewar (महेश बंडेवार)
> <maheshb@google.com> wrote:
> > On Thu, Sep 28, 2023 at 10:15 PM John Stultz <jstultz@google.com> wrote:
> > > 3) Nit: The interface is called ktime_get_cycles64 (timespec64
> > > returning interfaces usually are postfixed with ts64).
> > >
> > Ah, thanks for the explanation. I can change to comply with the
> > convention. Does ktime_get_cycles_ts64() make more sense?
>
> Maybe a little (it at least looks consistent), but not really if
> you're sticking raw cycles in the timespec :)
>

Despite my concerns that it's a bad idea, If one was going to expose
raw cycles from the timekeeping core, I'd suggest doing so directly as
a u64 (`u64 ktime_get_cycles(void)`).

That may mean widening (or maybe using a union in) your PTP ioctl data
structure to have a explicit cycles field.
Or introducing a separate ioctl that deals with cycles instead of timespec64s.

Squeezing data into types that are canonically used for something else
should always be avoided if possible (there are some cases where
you're stuck with an existing interface, but that's not the case
here).

But I still think we should avoid exporting the raw cycle values
unless there is some extremely strong argument for it (and if we can,
they should be abstracted into some sort of cookie value to avoid
userland using it as a raw clock).

thanks
-john