lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMuE1bG1ShKVakUH5f41_NNixBZRTUgXW8-=BWZxfNde6ohbVw@mail.gmail.com> Date: Thu, 30 Nov 2023 17:45:08 +0200 From: Sagi Maimon <maimon.sagi@...il.com> To: John Stultz <jstultz@...gle.com> Cc: Richard Cochran <richardcochran@...il.com>, reibax@...il.com, davem@...emloft.net, rrameshbabu@...dia.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, maheshb@...gle.com, Thomas Gleixner <tglx@...utronix.de> Subject: Re: [PATCH v2] posix-timers: add multi_clock_gettime system call Hi John, Thanks for your notes. On Tue, Nov 28, 2023 at 2:46 AM John Stultz <jstultz@...gle.com> wrote: > > On Mon, Nov 27, 2023 at 4:12 PM Richard Cochran > <richardcochran@...il.com> wrote: > > > > On Mon, Nov 27, 2023 at 05:39:01PM +0200, Sagi Maimon wrote: > > > Some user space applications need to read some clocks. > > > Each read requires moving from user space to kernel space. > > > This asymmetry causes the measured offset to have a significant error. > > > > Adding time/clock gurus (jstultz, tglx) on CC for visibility... > > > > Thanks for the heads up! (though, "guru" is just the noise I make > standing up these days) > > > > Introduce a new system call multi_clock_gettime, which can be used to measure > > > 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 clock > > > timestamp. > > This last bit about "offset includes the total time that the driver > needs to read the clock" is a bit confusing. It seems to suggest there > would be start/stop bookend timestamps so you could bound how long it > took to read all the clocks, but I don't see that in the patch. > You are right it is a little confusing, I will remove it from the commit . > > > 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 > > > > > > Signed-off-by: Sagi Maimon <maimon.sagi@...il.com> > > Overally, while I understand the intent, I'm pretty hesitant on it > (and "__ptp_multi_clock_get multi_clk_get" has me squinting to find > the actual space amongst all the underscores :). > struct __ptp_multi_clock_get __user * ptp_multi_clk_get has many underscores indeed :-) I will rename it, if you have any naming suggestions, I will be glad to hear it. > If the overhead of reading clockids individually is too much, it seems > like the next thing will be folks wanting to export multiple raw > hardware counter values so the counter->ns transformation doesn't get > inbetween each hw clock read, which this interface wouldn't solve, so > we'd have to add yet another interface. > future raw HW counter read interface: • For PHC - reading raw HW counter is driver dependent, and will require changes in many existing PTP drivers. • For System clock it is possible but with much more code changes and the improvements seems to be small. • The issue you introduced can be reduced (but not completely overcome) by user space usage of the interface. For example, to minimize the difference between clock X and clock Y, users can call multi_clock_gettime with 3 clock reads : X, Y, and X again. Considering gain (thin extra accuracy) vs pain (a lot of code changes, also for system clocks) and needs – we chose to settle with the current suggested interface. > Also, I wonder if trying to get multiple clocks in one read seems > similar to something uio_ring might help with? Though I can't say I'm > very savvy with uio_ring. Have folks looked into that? > I am also not an expert with the uio_ring/liburing, but I researched a little and it seems it is mainly used for IO operations and support only few of them (IORING_OP_READV, IORING_OP_SENDMSG, etc.) I am not sure how to implement it for consecutive clock_gettime system calls and if it can be done at all. Even if it can be done, it adds a lot of complexity to the user space code and the use in one generic system call is more elegant in my opinion. For example: Looking at the existing ioctl PTP_SYS_OFFSET, theoretically it can also be implemented by uio_ring, but it is still a more elegant solution. > thanks > -john
Powered by blists - more mailing lists