On 09/15/2015 05:49 PM, Arnd Bergmann wrote:
> The v4l2 event queue uses a 'struct timespec' to pass monotonic
> timestamps. This is not a problem by itself, but it breaks when user
> space redefines timespec to use 'long long' on 32-bit systems.
>
> To avoid that problem, we define our own replacement for timespec here,
> using 'long' tv_sec and tv_nsec members. This means no change to the
> existing API, but lets us keep using it with a y2038 compatible libc.
>
> As a side-effect, this changes the API for x32 programs to be the same
> as native 32-bit, using a 32-bit tv_sec/tv_nsec value, but both old and
> new kernels already support both formats for on the binary level for
> compat and x32.
>
> Alternatively, we could leave the header file to define the interface
> based on 'struct timespec', and implement both ioctls for native
> processes. I picked the approach in this case because it matches what
> we do for v4l2_timeval in the respective patch, but both would work
> equally well here.
>
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
> drivers/media/v4l2-core/v4l2-event.c | 20 +++++++++++++-------
> include/uapi/linux/videodev2.h | 8 +++++++-
> 2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-event.c b/drivers/media/v4l2-core/v4l2-event.c
> index 8d3171c6bee8..2a26ad591f59 100644
> --- a/drivers/media/v4l2-core/v4l2-event.c
> +++ b/drivers/media/v4l2-core/v4l2-event.c
> @@ -108,7 +108,7 @@ static struct v4l2_subscribed_event *v4l2_event_subscribed(
> }
>
> static void __v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev,
> - const struct timespec *ts)
> + const struct v4l2_timespec *ts)
> {
> struct v4l2_subscribed_event *sev;
> struct v4l2_kevent *kev;
> @@ -170,17 +170,20 @@ void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev)
> {
> struct v4l2_fh *fh;
> unsigned long flags;
> - struct timespec timestamp;
> + struct timespec64 timestamp;
> + struct v4l2_timespec vts;
>
> if (vdev == NULL)
> return;
>
> - ktime_get_ts(×tamp);
> + ktime_get_ts64(×tamp);
> + vts.tv_sec = timestamp.tv_sec;
> + vts.tv_nsec = timestamp.tv_nsec;
I prefer to take this opportunity to create a v4l2_get_timespec helper
function, just like v4l2_get_timeval.
>
> spin_lock_irqsave(&vdev->fh_lock, flags);
>
> list_for_each_entry(fh, &vdev->fh_list, list)
> - __v4l2_event_queue_fh(fh, ev, ×tamp);
> + __v4l2_event_queue_fh(fh, ev, &vts);
>
> spin_unlock_irqrestore(&vdev->fh_lock, flags);
> }
> @@ -189,12 +192,15 @@ EXPORT_SYMBOL_GPL(v4l2_event_queue);
> void v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev)
> {
> unsigned long flags;
> - struct timespec timestamp;
> + struct timespec64 timestamp;
> + struct v4l2_timespec vts;
>
> - ktime_get_ts(×tamp);
> + ktime_get_ts64(×tamp);
> + vts.tv_sec = timestamp.tv_sec;
> + vts.tv_nsec = timestamp.tv_nsec;
>
> spin_lock_irqsave(&fh->vdev->fh_lock, flags);
> - __v4l2_event_queue_fh(fh, ev, ×tamp);
> + __v4l2_event_queue_fh(fh, ev, &vts);
> spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
> }
> EXPORT_SYMBOL_GPL(v4l2_event_queue_fh);
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index b02cf054fbb8..bc659be87260 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -809,6 +809,12 @@ struct v4l2_timeval {
> long tv_usec;
> };
>
> +/* used for monotonic times, therefore y2038 safe */
> +struct v4l2_timespec {
> + long tv_sec;
> + long tv_nsec;
> +};
> +
> /**
> * struct v4l2_buffer - video buffer info
> * @index: id number of the buffer
> @@ -2088,7 +2094,7 @@ struct v4l2_event {
> } u;
> __u32 pending;
> __u32 sequence;
> - struct timespec timestamp;
> + struct v4l2_timespec timestamp;
> __u32 id;
> __u32 reserved[8];
> };
>
I think I am OK with this. This timestamp is used much more rarely and I do
not expect this ABI change to cause any problems in userspace.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
