On 09/16/2015 09:56 AM, Arnd Bergmann wrote:
> On Wednesday 16 September 2015 08:51:14 Hans Verkuil wrote:
>
>>> a) Similar to my first attempt, define a new struct v4l2_timeval, but
>>> only use it when building with a y2038-aware libc, so we don't break
>>> existing environments:
>>>
>>> /* some compile-time conditional that we first need to agree on with libc */
>>> #if __BITS_PER_TIME_T > __BITS_PER_LONG
>>> struct v4l2_timeval { long tv_sec; long tv_usec; }
>>> #else
>>> #define v4l2_timeval timeval
>>> #endif
>>>
>>> This means that any user space that currently assumes the timestamp
>>> member to be a 'struct timeval' has to be changed to access the members
>>> individually, or get a build error.
>>> The __BITS_PER_TIME_T trick has to be used in a couple of other subsystems
>>> too, as some of them have no other way to identify an interface
>>
>> I don't like this as this means some applications will compile on 64 bit or
>> with a non-y2038-aware libc, but fail on a 32-bit with y2038-aware libc. This
>> will be confusing and it may take a long time before the application developer
>> discovers this.
>
> Right.
>
>>> b) Keep the header file unchanged, but deal with both formats of v4l2_buffer
>>> in the kernel. Fortunately, all ioctls that pass a v4l2_buffer have
>>> properly defined command codes, and it does not get passed using a
>>> read/write style interface. This means we move the v4l2_buffer32
>>> handling from v4l2-compat-ioctl32.c to v4l2-ioctl.c and add an in-kernel
>>> v4l2_buffer64 that matches the 64-bit variant of v4l2_buffer.
>>> This way, user space can use either definition of time_t, and the
>>> kernel will just handle them natively.
>>> This is going to be the most common way to handle y2038 compatibility
>>> in device drivers, and it has the additional advantage of simplifying
>>> the compat path.
>>
>> This would work.
>
> Ok. So the only downside I can think of for this is that it uses a slightly
> less efficient format with additional padding in it. The kernel side will
> be a little ugly as I'm trying to avoid defining a generic timeval64
> structure (the generic syscalls should not need one), but I'll try to
> implement it first to see how it ends up.
>
>>> c) As you describe above, introduce a new v4l2_buffer replacement with
>>> a different layout that does not reference timeval. For this case, I
>>> would recommend using a single 64-bit nanosecond timestamp that can
>>> be generated using ktime_get_ns().
>>> However, to avoid ambiguity with the user space definition of struct
>>> timeval, we still have to hide the existing 'struct v4l2_buffer' from
>>> y2038-aware user space by enclosing it in '#if __BITS_PER_TIME_T >
>>> __BITS_PER_LONG' or similar.
>>
>> Right, and if we do that we still have the problem I describe under a). So we
>> would need to implement b) regardless.
>>
>> In other words, choosing c) doesn't depend on y2038 and it should be decided
>> on its own merits.
>>
>> I've proposed this as a topic to the media workshop we'll have during the Linux
>> Kernel Summit.
>
> Thanks, good idea. I'll be at the kernel summit, but don't plan to attend
> the media workshop otherwise. If you let me know about the schedule, I can
> come to this session (or ping me on IRC or hangout when it starts).
Are you also attending the ELCE in Dublin? We could have a quick talk there.
I think the discussion whether to switch to a new v4l2_buffer struct isn't really
dependent on anything y2038.
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
