Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
MIME-Version: 1.0
References: <20230929023737.1610865-1-maheshb@google.com> <CANDhNCqb5JzEDOdAnocanR2KFbokrpMOL=iNwY3fTxcn_ftuZQ@mail.gmail.com>
 <CAF2d9jgeGLCzbFZhptGzpUnmMgLaRysyzBmpZ+dK4sxWdmR5ZQ@mail.gmail.com> <CANDhNCro+AQum3eSmKK5OTNik2E0cFxV_reCQg0+_uTubHaDsA@mail.gmail.com>
In-Reply-To: <CANDhNCro+AQum3eSmKK5OTNik2E0cFxV_reCQg0+_uTubHaDsA@mail.gmail.com>
From:   John Stultz <jstultz@google.com>
Date:   Fri, 29 Sep 2023 00:06:46 -0700
Message-ID: <CANDhNCryn8TjJZRdCvVUj88pakHSUvtyN53byjmAcyowKj5mcA@mail.gmail.com>
Subject: Re: [PATCH 1/4] time: add ktime_get_cycles64() api
To:     =?UTF-8?B?TWFoZXNoIEJhbmRld2FyICjgpK7gpLngpYfgpLYg4KSs4KSC4KSh4KWH4KS14KS+4KSwKQ==?= 
        <maheshb@google.com>
Cc:     Netdev <netdev@vger.kernel.org>,
        Linux <linux-kernel@vger.kernel.org>,
        David Miller <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Don Hatchett <hatch@google.com>,
        Yuliang Li <yuliangli@google.com>,
        Mahesh Bandewar <mahesh@bandewar.net>,
        Thomas Gleixner <tglx@linutronix.de>,
        Stephen Boyd <sboyd@kernel.org>,
        Richard Cochran <richardcochran@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Sep 28, 2023 at 11:56=E2=80=AFPM John Stultz <jstultz@google.com> w=
rote:
> On Thu, Sep 28, 2023 at 11:35=E2=80=AFPM Mahesh Bandewar (=E0=A4=AE=E0=A4=
=B9=E0=A5=87=E0=A4=B6 =E0=A4=AC=E0=A4=82=E0=A4=A1=E0=A5=87=E0=A4=B5=E0=A4=
=BE=E0=A4=B0)
> <maheshb@google.com> wrote:
> > On Thu, Sep 28, 2023 at 10:15=E2=80=AFPM John Stultz <jstultz@google.co=
m> 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 timespec6=
4s.

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