All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
@ 2019-09-23 13:05 Eugene Syromiatnikov
  2020-05-12 19:58 ` Sergey Organov
  0 siblings, 1 reply; 7+ messages in thread
From: Eugene Syromiatnikov @ 2019-09-23 13:05 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner, Stephen Boyd
  Cc: Frederic Weisbecker, Sebastian Andrzej Siewior,
	Eric W. Biederman, linux-kernel

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 <esyr@redhat.com>
---
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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
  2019-09-23 13:05 [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI Eugene Syromiatnikov
@ 2020-05-12 19:58 ` Sergey Organov
  2020-05-12 22:31   ` Eugene Syromiatnikov
  0 siblings, 1 reply; 7+ messages in thread
From: Sergey Organov @ 2020-05-12 19:58 UTC (permalink / raw)
  To: Eugene Syromiatnikov
  Cc: John Stultz, Thomas Gleixner, Stephen Boyd, Frederic Weisbecker,
	Sebastian Andrzej Siewior, Eric W. Biederman, linux-kernel

Eugene Syromiatnikov <esyr@redhat.com> 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 <esyr@redhat.com>
> ---
> 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
  2020-05-12 19:58 ` Sergey Organov
@ 2020-05-12 22:31   ` Eugene Syromiatnikov
  2020-05-12 22:40     ` John Stultz
  0 siblings, 1 reply; 7+ messages in thread
From: Eugene Syromiatnikov @ 2020-05-12 22:31 UTC (permalink / raw)
  To: Sergey Organov
  Cc: John Stultz, Thomas Gleixner, Stephen Boyd, Frederic Weisbecker,
	Sebastian Andrzej Siewior, Eric W. Biederman, linux-kernel

On Tue, May 12, 2020 at 10:58:16PM +0300, Sergey Organov wrote:
> Eugene Syromiatnikov <esyr@redhat.com> 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 <esyr@redhat.com>
> > ---
> > 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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
  2020-05-12 22:31   ` Eugene Syromiatnikov
@ 2020-05-12 22:40     ` John Stultz
  2020-05-13  9:13       ` Sergey Organov
  0 siblings, 1 reply; 7+ messages in thread
From: John Stultz @ 2020-05-12 22:40 UTC (permalink / raw)
  To: Eugene Syromiatnikov
  Cc: Sergey Organov, Thomas Gleixner, Stephen Boyd,
	Frederic Weisbecker, Sebastian Andrzej Siewior,
	Eric W. Biederman, lkml

On Tue, May 12, 2020 at 3:31 PM Eugene Syromiatnikov <esyr@redhat.com> wrote:
> On Tue, May 12, 2020 at 10:58:16PM +0300, Sergey Organov wrote:
> > Eugene Syromiatnikov <esyr@redhat.com> 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 <esyr@redhat.com>
> > > ---
> > > 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
  2020-05-12 22:40     ` John Stultz
@ 2020-05-13  9:13       ` Sergey Organov
  2020-05-13 17:11         ` John Stultz
  0 siblings, 1 reply; 7+ messages in thread
From: Sergey Organov @ 2020-05-13  9:13 UTC (permalink / raw)
  To: John Stultz
  Cc: Eugene Syromiatnikov, Thomas Gleixner, Stephen Boyd,
	Frederic Weisbecker, Sebastian Andrzej Siewior,
	Eric W. Biederman, lkml

John Stultz <john.stultz@linaro.org> writes:

> On Tue, May 12, 2020 at 3:31 PM Eugene Syromiatnikov <esyr@redhat.com> wrote:
>> On Tue, May 12, 2020 at 10:58:16PM +0300, Sergey Organov wrote:
>> > Eugene Syromiatnikov <esyr@redhat.com> 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 <esyr@redhat.com>
>> > > ---
>> > > 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
  2020-05-13  9:13       ` Sergey Organov
@ 2020-05-13 17:11         ` John Stultz
  2020-05-13 20:28           ` Sergey Organov
  0 siblings, 1 reply; 7+ messages in thread
From: John Stultz @ 2020-05-13 17:11 UTC (permalink / raw)
  To: Sergey Organov
  Cc: Eugene Syromiatnikov, Thomas Gleixner, Stephen Boyd,
	Frederic Weisbecker, Sebastian Andrzej Siewior,
	Eric W. Biederman, lkml

On Wed, May 13, 2020 at 2:13 AM Sergey Organov <sorganov@gmail.com> wrote:
>
> John Stultz <john.stultz@linaro.org> writes:
>
> > On Tue, May 12, 2020 at 3:31 PM Eugene Syromiatnikov <esyr@redhat.com> wrote:
> >> On Tue, May 12, 2020 at 10:58:16PM +0300, Sergey Organov wrote:
> >> > Eugene Syromiatnikov <esyr@redhat.com> 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 <esyr@redhat.com>
> >> > > ---
> >> > > 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI
  2020-05-13 17:11         ` John Stultz
@ 2020-05-13 20:28           ` Sergey Organov
  0 siblings, 0 replies; 7+ messages in thread
From: Sergey Organov @ 2020-05-13 20:28 UTC (permalink / raw)
  To: John Stultz
  Cc: Eugene Syromiatnikov, Thomas Gleixner, Stephen Boyd,
	Frederic Weisbecker, Sebastian Andrzej Siewior,
	Eric W. Biederman, lkml

John Stultz <john.stultz@linaro.org> writes:

> On Wed, May 13, 2020 at 2:13 AM Sergey Organov <sorganov@gmail.com> wrote:
>>
>> John Stultz <john.stultz@linaro.org> writes:
>>
>> > On Tue, May 12, 2020 at 3:31 PM Eugene Syromiatnikov
>> > <esyr@redhat.com> wrote:
>> >> On Tue, May 12, 2020 at 10:58:16PM +0300, Sergey Organov wrote:
>> >> > Eugene Syromiatnikov <esyr@redhat.com> 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 <esyr@redhat.com>
>> >> > > ---
>> >> > > 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-05-13 20:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-23 13:05 [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI Eugene Syromiatnikov
2020-05-12 19:58 ` Sergey Organov
2020-05-12 22:31   ` Eugene Syromiatnikov
2020-05-12 22:40     ` John Stultz
2020-05-13  9:13       ` Sergey Organov
2020-05-13 17:11         ` John Stultz
2020-05-13 20:28           ` Sergey Organov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.