Re: [PATCH v2] uapi, posix-timers: provide clockid-related macros and functions to UAPI

[Sergey Organov <sorganov@gmail.com>]

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