John Stultz <[email protected]> writes: > On Wed, May 13, 2020 at 2:13 AM Sergey Organov <[email protected]> wrote: >> >> John Stultz <[email protected]> writes: >> >> > On Tue, May 12, 2020 at 3:31 PM Eugene Syromiatnikov >> > <[email protected]> wrote: >> >> On Tue, May 12, 2020 at 10:58:16PM +0300, Sergey Organov wrote: >> >> > Eugene Syromiatnikov <[email protected]> writes: >> >> > >> >> > > As of now, there is no interface exposed for converting pid/fd into >> >> > > clockid and vice versa; linuxptp, for example, has been carrying these >> >> > > definitions in missing.h header for quite some time[1]. >> >> > > >> >> > > [1] https://sourceforge.net/p/linuxptp/code/ci/af380e86/tree/missing.h >> >> > > >> >> > > Signed-off-by: Eugene Syromiatnikov <[email protected]> >> >> > > --- >> >> > > Changes since v1[1]: >> >> > > * Actually tried to build with the patch and fixed the build error >> >> > > reported by kbuild test robot[2]. >> >> > > >> >> > > [1] https://lkml.org/lkml/2019/9/20/698 >> >> > > [2] https://lkml.org/lkml/2019/9/22/13 >> >> > > --- >> >> > > include/linux/posix-timers.h | 47 >> >> > > +------------------------------------------ >> >> > > include/uapi/linux/time.h | 48 >> >> > > ++++++++++++++++++++++++++++++++++++++++++++ >> >> > > 2 files changed, 49 insertions(+), 46 deletions(-) >> >> > >> >> > Was this patch applied, rejected, lost? >> >> > >> >> > I can't find it in the current master. >> >> >> >> IIRC, it was ignored. >> > >> > Overlooked. :) Not intentionally ignored. >> > >> > I don't have any major objection with adding helpers, though I feel >> > like you're exporting a lot more to the uapi then applications likely >> > need. >> > >> > Would it be better to add just the bits from the missing.h header you >> > pointed to: >> > #define CLOCKFD 3 >> > #define FD_TO_CLOCKID(fd) ((~(clockid_t) (fd) << 3) | CLOCKFD) >> > #define CLOCKID_TO_FD(clk) ((unsigned int) ~((clk) >> 3)) >> > >> > to the uapi header? >> >> Please, no: >> >> 1. These macros were copied almost verbatim from the kernel code long >> ago, and since then kernel has changed them to inline functions, so >> getting back to these obsolete macros is pointless. >> >> 2. If we do need to export macroses, then kernel inline functions are >> better to be re-implemented in terms of these macros, not to have 2 >> different points of implementation. >> >> Overall, I'd vote for the current approach of the patch, provided >> exporting inline functions to user-space is allowed. > > Sure, I just want to make sure we're only exporting the minimal > necessary amount of details to userland. The current patch is > exporting a bit more than that.
>From userland POV, I've only seen 2 above conversions to be used, and I have absolutely no idea if the other 2 functions: static inline __kernel_clockid_t make_process_cpuclock(const unsigned int pid, static inline __kernel_clockid_t make_thread_cpuclock(const unsigned int tid, are directly useful from userspace. Then, I now realize that exporting defines could be a better idea as their existence could be easily checked from userspace. However, exporting exactly these defines would likely break existing userspace due to redefinition, so we probably need to come-up with different names then. And then it's probably C library that ideally should provide corresponding interface to user programs, so there is yet another layer to be considered? Personally, I don't feel being experienced enough in kernel-to-userspace interface subtleties to suggest proper patch that'd expose minimum details yet doesn't create too much of maintenance burden both in the kernel and userspace. Thanks, -- Sergey
