Received-SPF: pass (google.com: domain of linux-kernel+bounces-13840-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20231231170721.3381-1-maimon.sagi@gmail.com> <CALCETrUd=16gAYvx93EsyMaaSJ-6mLvSru8Gie48Y+_dXq5FGA@mail.gmail.com>
In-Reply-To: <CALCETrUd=16gAYvx93EsyMaaSJ-6mLvSru8Gie48Y+_dXq5FGA@mail.gmail.com>
From: Sagi Maimon <maimon.sagi@gmail.com>
Date: Mon, 1 Jan 2024 10:44:55 +0200
Message-ID: <CAMuE1bEuou1Bx-6c3es3+FuTguOCb+iqU=hickK5hP7wT=M6Pw@mail.gmail.com>
Subject: Re: [PATCH v4] posix-timers: add multi_clock_gettime system call
To: Andy Lutomirski <luto@kernel.org>
Cc: richardcochran@gmail.com, datglx@linutronix.de, mingo@redhat.com, 
	bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, 
	arnd@arndb.de, geert@linux-m68k.org, peterz@infradead.org, hannes@cmpxchg.org, 
	sohil.mehta@intel.com, rick.p.edgecombe@intel.com, nphamcs@gmail.com, 
	palmer@sifive.com, keescook@chromium.org, legion@kernel.org, 
	mark.rutland@arm.com, linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, 
	linux-arch@vger.kernel.org, netdev@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sun, Dec 31, 2023 at 11:10=E2=80=AFPM Andy Lutomirski <luto@kernel.org> =
wrote:
>
> On Sun, Dec 31, 2023 at 9:07=E2=80=AFAM Sagi Maimon <maimon.sagi@gmail.co=
m> wrote:
> >
> > Some user space applications need to read some clocks.
> > Each read requires moving from user space to kernel space.
> > The syscall overhead causes unpredictable delay between N clocks reads
> > Removing this delay causes better synchronization between N clocks.
> >
> > Introduce a new system call multi_clock_gettime, which can be used to m=
easure
> > the offset between multiple clocks, from variety of types: PHC, virtual=
 PHC
> > and various system clocks (CLOCK_REALTIME, CLOCK_MONOTONIC, etc).
> > The offset includes the total time that the driver needs to read the cl=
ock
> > timestamp.
Andy Thank you for your notes.
>
> Knowing this offset sounds quite nice, but...
>
> >
> > New system call allows the reading of a list of clocks - up to PTP_MAX_=
CLOCKS.
> > Supported clocks IDs: PHC, virtual PHC and various system clocks.
> > Up to PTP_MAX_SAMPLES times (per clock) in a single system call read.
> > The system call returns n_clocks timestamps for each measurement:
> > - clock 0 timestamp
> > - ...
> > - clock n timestamp
>
> Could this instead be arranged to read the actual, exact offset?
>
It can be done, but I prefer to leave it generic and consistent with
other time system calls.
In most cases the offset calculation is done in user space application
> > +       kernel_tp =3D kernel_tp_base;
> > +       for (j =3D 0; j < n_samples; j++) {
> > +               for (i =3D 0; i < n_clocks; i++) {
> > +                       if (put_timespec64(kernel_tp++, (struct __kerne=
l_timespec __user *)
> > +                                       &ptp_multi_clk_get->ts[j][i])) =
{
> > +                               error =3D -EFAULT;
> > +                               goto out;
> > +                       }
> > +               }
> > +       }
>
> There are several pairs of clocks that tick at precisely same rate
> (and use the same underlying hardware clock), and the offset could be
> computed exactly instead of doing this noisy loop that is merely
> somewhat less bad than what user code could do all by itself.
You are correct, there are some PHCs on the same NIC (each per port)
that share the same HW counter/clock.
In that case it is slightly better to do the offset calculation in the
NIC driver code,
but that requires changes in each NIC driver's code.
The main thing is that the multi_clock_gettime system call is a
generic solution,
it covers that case among other cases, for example sync between two
PHCs on different NICs.