Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
John Stultz writes: > On Wed, May 13, 2020 at 2:13 AM Sergey Organov wrote: >> >> John Stultz writes: >> >> > On Tue, May 12, 2020 at 3:31 PM Eugene Syromiatnikov >> > wrote: >> >> On Tue, May 12, 2020 at 10:58:16PM +0300, Sergey Organov wrote: >> >> > Eugene Syromiatnikov 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 >> >> > > --- >> >> > > 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
Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
On Wed, May 13, 2020 at 2:13 AM Sergey Organov wrote: > > John Stultz writes: > > > On Tue, May 12, 2020 at 3:31 PM Eugene Syromiatnikov > > wrote: > >> On Tue, May 12, 2020 at 10:58:16PM +0300, Sergey Organov wrote: > >> > Eugene Syromiatnikov 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 > >> > > --- > >> > > 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. thanks -john
Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
John Stultz writes: > On Tue, May 12, 2020 at 3:31 PM Eugene Syromiatnikov wrote: >> On Tue, May 12, 2020 at 10:58:16PM +0300, Sergey Organov wrote: >> > Eugene Syromiatnikov 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 >> > > --- >> > > 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. -- Sergey Organov
Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
On Tue, May 12, 2020 at 3:31 PM Eugene Syromiatnikov wrote: > On Tue, May 12, 2020 at 10:58:16PM +0300, Sergey Organov wrote: > > Eugene Syromiatnikov 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 > > > --- > > > 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? thanks -john
Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
On Tue, May 12, 2020 at 10:58:16PM +0300, Sergey Organov wrote: > Eugene Syromiatnikov 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 > > --- > > 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.
Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
Eugene Syromiatnikov 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 > --- > 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. Thanks, -- Sergey Organov
[PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
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 --- 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(-) diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 3d10c84..ddc7e6e6 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -4,58 +4,13 @@ #include #include +#include #include #include struct kernel_siginfo; struct task_struct; -/* - * Bit fields within a clockid: - * - * The most significant 29 bits hold either a pid or a file descriptor. - * - * Bit 2 indicates whether a cpu clock refers to a thread or a process. - * - * Bits 1 and 0 give the type: PROF=0, VIRT=1, SCHED=2, or FD=3. - * - * A clockid is invalid if bits 2, 1, and 0 are all set. - */ -#define CPUCLOCK_PID(clock)((pid_t) ~((clock) >> 3)) -#define CPUCLOCK_PERTHREAD(clock) \ - (((clock) & (clockid_t) CPUCLOCK_PERTHREAD_MASK) != 0) - -#define CPUCLOCK_PERTHREAD_MASK4 -#define CPUCLOCK_WHICH(clock) ((clock) & (clockid_t) CPUCLOCK_CLOCK_MASK) -#define CPUCLOCK_CLOCK_MASK3 -#define CPUCLOCK_PROF 0 -#define CPUCLOCK_VIRT 1 -#define CPUCLOCK_SCHED 2 -#define CPUCLOCK_MAX 3 -#define CLOCKFDCPUCLOCK_MAX -#define CLOCKFD_MASK (CPUCLOCK_PERTHREAD_MASK|CPUCLOCK_CLOCK_MASK) - -static inline clockid_t make_process_cpuclock(const unsigned int pid, - const clockid_t clock) -{ - return ((~pid) << 3) | clock; -} -static inline clockid_t make_thread_cpuclock(const unsigned int tid, - const clockid_t clock) -{ - return make_process_cpuclock(tid, clock | CPUCLOCK_PERTHREAD_MASK); -} - -static inline clockid_t fd_to_clockid(const int fd) -{ - return make_process_cpuclock((unsigned int) fd, CLOCKFD); -} - -static inline int clockid_to_fd(const clockid_t clk) -{ - return ~(clk >> 3); -} - #ifdef CONFIG_POSIX_TIMERS /** diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h index 958932e..58a78a7 100644 --- a/include/uapi/linux/time.h +++ b/include/uapi/linux/time.h @@ -70,4 +70,52 @@ struct itimerval { */ #define TIMER_ABSTIME 0x01 +/* + * Bit fields within a clockid: + * + * The most significant 29 bits hold either a pid or a file descriptor. + * + * Bit 2 indicates whether a cpu clock refers to a thread or a process. + * + * Bits 1 and 0 give the type: PROF=0, VIRT=1, SCHED=2, or FD=3. + * + * A clockid is invalid if bits 2, 1, and 0 are all set. + */ +#define CPUCLOCK_PID(clock)((pid_t) ~((clock) >> 3)) +#define CPUCLOCK_PERTHREAD(clock) \ + (((clock) & (__kernel_clockid_t) CPUCLOCK_PERTHREAD_MASK) != 0) + +#define CPUCLOCK_PERTHREAD_MASK4 +#define CPUCLOCK_WHICH(clock) \ + ((clock) & (__kernel_clockid_t) CPUCLOCK_CLOCK_MASK) +#define CPUCLOCK_CLOCK_MASK3 +#define CPUCLOCK_PROF 0 +#define CPUCLOCK_VIRT 1 +#define CPUCLOCK_SCHED 2 +#define CPUCLOCK_MAX 3 +#define CLOCKFDCPUCLOCK_MAX +#define CLOCKFD_MASK (CPUCLOCK_PERTHREAD_MASK|CPUCLOCK_CLOCK_MASK) + +static inline __kernel_clockid_t make_process_cpuclock(const unsigned int pid, + const clockid_t clock) +{ + return ((~pid) << 3) | clock; +} +static inline __kernel_clockid_t make_thread_cpuclock(const unsigned int tid, + const clockid_t clock) +{ + return make_process_cpuclock(tid, clock | CPUCLOCK_PERTHREAD_MASK); +} + +static inline __kernel_clockid_t fd_to_clockid(const int fd) +{ + return make_process_cpuclock((unsigned int) fd, CLOCKFD); +} + +static inline int clockid_to_fd(const __kernel_clockid_t clk) +{ + return ~(clk >> 3); +} + + #endif /* _UAPI_LINUX_TIME_H */ -- 2.1.4