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(-)
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 <linux/spinlock.h>
#include <linux/list.h>
+#include <linux/time.h>
#include <linux/alarmtimer.h>
#include <linux/timerqueue.h>
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_MASK 4
-#define CPUCLOCK_WHICH(clock) ((clock) & (clockid_t) CPUCLOCK_CLOCK_MASK)
-#define CPUCLOCK_CLOCK_MASK 3
-#define CPUCLOCK_PROF 0
-#define CPUCLOCK_VIRT 1
-#define CPUCLOCK_SCHED 2
-#define CPUCLOCK_MAX 3
-#define CLOCKFD CPUCLOCK_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_MASK 4
+#define CPUCLOCK_WHICH(clock) \
+ ((clock) & (__kernel_clockid_t) CPUCLOCK_CLOCK_MASK)
+#define CPUCLOCK_CLOCK_MASK 3
+#define CPUCLOCK_PROF 0
+#define CPUCLOCK_VIRT 1
+#define CPUCLOCK_SCHED 2
+#define CPUCLOCK_MAX 3
+#define CLOCKFD CPUCLOCK_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
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.
Thanks,
-- Sergey Organov
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.
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?
thanks
-john
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.
-- Sergey Organov
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.
thanks
-john
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