Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-31 Thread Peter Hutterer 
On Sat, Oct 29, 2016 at 09:19:05PM -0700, Deepa Dinamani wrote:
> > btw, where did you post the libevdev patch? I haven't seen it anywhere I'm
> > subscribed to.
> 
> The libevdev patch was posted to input-to...@lists.freedesktop.org :
> https://www.mail-archive.com/y2038@lists.linaro.org/msg01824.html

ah, right. that would do it.  input-tools@ discarded all posts by nonmembers
(guess that explains why we didn't get much spam :)

I changed that over now, please re-send to the list, thanks.

Cheers,
   Peter

___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-29 Thread Deepa Dinamani 
> I think we should do those two things completely independently.
> We need to do something now to preserve the current interfaces
> for the glibc changes that are coming soon [1], and Deepa's
> patches do that (though I now realize the changelog doesn't
> mention the requirement).

I'll update the changelog in the next version of the series along with
the task check for x32 mode.

> An overhaul of the input_event handling with a new modern
> but incompatible format may or may not be a good idea, but
> this should be decided independently.

Just to clarify, we are going with the plan that Arnd suggested:
leaving the overhaul of the input event protocol alone and proceeding
with the series according to suggested changes.
Are we in agreement over this?

-Deepa
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-29 Thread Deepa Dinamani 
> btw, where did you post the libevdev patch? I haven't seen it anywhere I'm
> subscribed to.

The libevdev patch was posted to input-to...@lists.freedesktop.org :
https://www.mail-archive.com/y2038@lists.linaro.org/msg01824.html


-Deepa
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-28 Thread Arnd Bergmann 
On Friday, October 28, 2016 2:56:10 PM CEST Dmitry Torokhov wrote:
> On Fri, Oct 28, 2016 at 2:47 PM, Arnd Bergmann  wrote:
> > On Friday, October 28, 2016 2:39:35 PM CEST Deepa Dinamani wrote:
> >> >> >> @@ -55,24 +60,24 @@ struct ff_effect_compat {
> >> >> >>
> >> >> >>  static inline size_t input_event_size(void)
> >> >> >>  {
> >> >> >> -   return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
> >> >> >> -   sizeof(struct input_event_compat) : sizeof(struct 
> >> >> >> input_event);
> >> >> >> +   return in_compat_syscall() ? sizeof(struct 
> >> >> >> raw_input_event_compat) :
> >> >> >> +sizeof(struct raw_input_event);
> >> >> >>  }
> >> >> >
> >> >> > I think the COMPAT_USE_64BIT_TIME check has to stay here,
> >> >> > it's needed for x32 mode on x86-64.
> >> >>
> >
> > We have to distinguish four cases on x86:
> >
> > - native 32-bit, input_event with 32-bit time_t
> > - compat 32-bit, input_event_compat with 32-bit time_t
> > - native 64-bit, input_event with 64-bit time_t
> > - compat x32, input_event with 64-bit time_t
> >
> > The first three can happen on other architectures too,
> > the last one is x86 specific. There are probably other ways
> > to express the condition above, but I can't think of one
> > that is better than the one we have today.
> 
> Can we detect if given task is compat x32, like we do for compat
> 64/32? Or entire userspace has to be x32?

Yes, this works fine per task, with the definition of COMPAT_USE_64BIT_TIME
that is hardcoded to zero everywhere except on x86 where it is

#define COMPAT_USE_64BIT_TIME \
(!!(task_pt_regs(current)->orig_ax & __X32_SYSCALL_BIT))

This is unrelated to the patch in question, the existing code
is correct as long as we don't change the logic and just
replace input_event with raw_input_event (or __kernel_input_event
or whichever you prefer).

Arnd
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-28 Thread Dmitry Torokhov 
On Fri, Oct 28, 2016 at 2:47 PM, Arnd Bergmann  wrote:
> On Friday, October 28, 2016 2:39:35 PM CEST Deepa Dinamani wrote:
>> >> >> @@ -55,24 +60,24 @@ struct ff_effect_compat {
>> >> >>
>> >> >>  static inline size_t input_event_size(void)
>> >> >>  {
>> >> >> -   return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
>> >> >> -   sizeof(struct input_event_compat) : sizeof(struct 
>> >> >> input_event);
>> >> >> +   return in_compat_syscall() ? sizeof(struct 
>> >> >> raw_input_event_compat) :
>> >> >> +sizeof(struct raw_input_event);
>> >> >>  }
>> >> >
>> >> > I think the COMPAT_USE_64BIT_TIME check has to stay here,
>> >> > it's needed for x32 mode on x86-64.
>> >>
>> >> There is no time_t anymore in the raw_input_event structure.
>> >> The struct uses __kernel_ulong_t type.
>> >> This should take care of x32 support.
>> >
>> > I don't think it does.
>> >
>> >> From this cover letter:
>> >> https://www.spinics.net/lists/linux-arch/msg16356.html
>> >>
>> >> I see that that the __kernel types were introduced to address the ABI
>> >> issues for x32.
>> >
>> > This is a variation of the problem we are trying to solve for
>> > the other architectures in your patch set:
>> >
>> > On x32, the kernel uses produces a structure with the 64-bit
>> > layout, using __u64 tv_sec, to match the current user space
>> > that has 64-bit __kernel_ulong_t and 64-bit time_t, but
>> > in_compat_syscall() also returns 'true' here, as this is
>> > mostly a 32-bit ABI (time_t being one of the exceptions).
>>
>> Yes, I missed this.
>>
>> in_compat_syscall() is true for x32, this would mean we end up here
>> even if it is a x32 syscall.
>> But, wouldn't it be better to use in_x32_syscall() here since there is
>> no timeval any more?
>
> We have to distinguish four cases on x86:
>
> - native 32-bit, input_event with 32-bit time_t
> - compat 32-bit, input_event_compat with 32-bit time_t
> - native 64-bit, input_event with 64-bit time_t
> - compat x32, input_event with 64-bit time_t
>
> The first three can happen on other architectures too,
> the last one is x86 specific. There are probably other ways
> to express the condition above, but I can't think of one
> that is better than the one we have today.

Can we detect if given task is compat x32, like we do for compat
64/32? Or entire userspace has to be x32?

Thanks.

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-28 Thread Arnd Bergmann 
On Friday, October 28, 2016 2:39:35 PM CEST Deepa Dinamani wrote:
> >> >> @@ -55,24 +60,24 @@ struct ff_effect_compat {
> >> >>
> >> >>  static inline size_t input_event_size(void)
> >> >>  {
> >> >> -   return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
> >> >> -   sizeof(struct input_event_compat) : sizeof(struct 
> >> >> input_event);
> >> >> +   return in_compat_syscall() ? sizeof(struct 
> >> >> raw_input_event_compat) :
> >> >> +sizeof(struct raw_input_event);
> >> >>  }
> >> >
> >> > I think the COMPAT_USE_64BIT_TIME check has to stay here,
> >> > it's needed for x32 mode on x86-64.
> >>
> >> There is no time_t anymore in the raw_input_event structure.
> >> The struct uses __kernel_ulong_t type.
> >> This should take care of x32 support.
> >
> > I don't think it does.
> >
> >> From this cover letter:
> >> https://www.spinics.net/lists/linux-arch/msg16356.html
> >>
> >> I see that that the __kernel types were introduced to address the ABI
> >> issues for x32.
> >
> > This is a variation of the problem we are trying to solve for
> > the other architectures in your patch set:
> >
> > On x32, the kernel uses produces a structure with the 64-bit
> > layout, using __u64 tv_sec, to match the current user space
> > that has 64-bit __kernel_ulong_t and 64-bit time_t, but
> > in_compat_syscall() also returns 'true' here, as this is
> > mostly a 32-bit ABI (time_t being one of the exceptions).
> 
> Yes, I missed this.
> 
> in_compat_syscall() is true for x32, this would mean we end up here
> even if it is a x32 syscall.
> But, wouldn't it be better to use in_x32_syscall() here since there is
> no timeval any more?

We have to distinguish four cases on x86:

- native 32-bit, input_event with 32-bit time_t
- compat 32-bit, input_event_compat with 32-bit time_t
- native 64-bit, input_event with 64-bit time_t
- compat x32, input_event with 64-bit time_t

The first three can happen on other architectures too,
the last one is x86 specific. There are probably other ways
to express the condition above, but I can't think of one
that is better than the one we have today.

Arnd
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-28 Thread Arnd Bergmann 
On Friday, October 28, 2016 8:19:46 AM CEST Deepa Dinamani wrote:
> On Fri, Oct 28, 2016 at 5:43 AM, Arnd Bergmann  wrote:
> > On Monday, October 17, 2016 8:27:32 PM CEST Deepa Dinamani wrote:
> >> @@ -55,24 +60,24 @@ struct ff_effect_compat {
> >>
> >>  static inline size_t input_event_size(void)
> >>  {
> >> -   return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
> >> -   sizeof(struct input_event_compat) : sizeof(struct 
> >> input_event);
> >> +   return in_compat_syscall() ? sizeof(struct raw_input_event_compat) 
> >> :
> >> +sizeof(struct raw_input_event);
> >>  }
> >
> > I think the COMPAT_USE_64BIT_TIME check has to stay here,
> > it's needed for x32 mode on x86-64.
> 
> There is no time_t anymore in the raw_input_event structure.
> The struct uses __kernel_ulong_t type.
> This should take care of x32 support.

I don't think it does.

> From this cover letter:
> https://www.spinics.net/lists/linux-arch/msg16356.html
> 
> I see that that the __kernel types were introduced to address the ABI
> issues for x32.

This is a variation of the problem we are trying to solve for
the other architectures in your patch set:

On x32, the kernel uses produces a structure with the 64-bit
layout, using __u64 tv_sec, to match the current user space
that has 64-bit __kernel_ulong_t and 64-bit time_t, but
in_compat_syscall() also returns 'true' here, as this is
mostly a 32-bit ABI (time_t being one of the exceptions).

ARnd
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-28 Thread Deepa Dinamani 
On Fri, Oct 28, 2016 at 5:43 AM, Arnd Bergmann  wrote:
> On Monday, October 17, 2016 8:27:32 PM CEST Deepa Dinamani wrote:
>> @@ -55,24 +60,24 @@ struct ff_effect_compat {
>>
>>  static inline size_t input_event_size(void)
>>  {
>> -   return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
>> -   sizeof(struct input_event_compat) : sizeof(struct 
>> input_event);
>> +   return in_compat_syscall() ? sizeof(struct raw_input_event_compat) :
>> +sizeof(struct raw_input_event);
>>  }
>
> I think the COMPAT_USE_64BIT_TIME check has to stay here,
> it's needed for x32 mode on x86-64.

There is no time_t anymore in the raw_input_event structure.
The struct uses __kernel_ulong_t type.
This should take care of x32 support.

From this cover letter:
https://www.spinics.net/lists/linux-arch/msg16356.html

I see that that the __kernel types were introduced to address the ABI
issues for x32.

-Deepa
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-28 Thread Arnd Bergmann 
On Friday, October 28, 2016 2:46:42 PM CEST Peter Hutterer wrote:
> On Thu, Oct 27, 2016 at 03:24:55PM -0700, Deepa Dinamani wrote:
> > On Wed, Oct 26, 2016 at 7:56 PM, Peter Hutterer
> >  wrote:
> > > On Mon, Oct 17, 2016 at 08:27:32PM -0700, Deepa Dinamani wrote:
> > > general comment here - please don't name it "raw_input_event".
> > > First, when you grep for input_event you want the new ones to show up too,
> > > so a struct input_event_raw would be better here. That also has better
> > > namespacing in general. Second though: the event isn't any more "raw" than
> > > the previous we had.
> > >
> > > I can't think of anything better than struct input_event_v2 though.
> > 
> > The general idea was to leave the original struct input_event as a
> > common interface for userspace (as it cannot be deleted).
> > So reading raw data unformatted by the userspace will have the new
> > struct raw_input_event format.
> > This was the reason for the "raw" in the name.
> > 
> > struct input_event_v2 is fine too, if this is more preferred.

I think input_event_v2 would be more confusing. An alternative
to raw_input_event might be __kernel_input_event, which parallels
things like __kernel_off_t that is also independent from the user
space off_t in the same way that the user space input_event
structure will get redefined in a way that is incompatible with
the kernel ABI.

> > >> This replaces timeval with struct input_timeval. This structure
> > >> maintains time in __kernel_ulong_t or compat_ulong_t to allow
> > >> for architectures to override types as in the case of x32.
> > >>
> > >> The change requires any userspace utilities reading or writing
> > >> from event nodes to update their reading format to match
> > >> raw_input_event. The changes to the popular libraries will be
> > >> posted along with the kernel changes.
> > >> The driver version is also updated to reflect the change in
> > >> event format.
> > >
> > > Doesn't this break *all* of userspace then? I don't see anything to
> > > negotiate the type of input event the kernel gives me. And nothing right 
> > > now
> > > checks for EVDEV_VERSION, so they all just assume it's a struct
> > > input_event. Best case, if the available events aren't a multiple of
> > > sizeof(struct input_event) userspace will bomb out, but unless that 
> > > happens,
> > > everyone will just happily read old-style events.
> > >
> > > So we need some negotiation what is acceptable. Which also needs to 
> > > address
> > > the race conditions we're going to get when events start coming in before
> > > the client has announced that it supports the new-style events.
> > 
> > No, this does not break any userspace right now.
> > Both struct input_event and struct raw_input_event are exactly the same 
> > today.
> 
> oh, right, the ABI is the same. I see that now, thanks.

One minor difference is that the seconds in raw_input_event are
'unsigned', so even with the 'real' time domain, we can represent
times from 1970 to 2106, whereas 'timeval' represents times between
1902 and 2038.

Once user space has a 64-bit time_t and the conversion function
in libinput that Deepa suggested, the raw_input_event is only
used on the kernel ABI and the normal timestamps will work fine.

> > And, hence any library that results in a call to libevdev_set_fd()
> > will fail if it is not this updated driver.
> 
> without having seen the libevdev patch - that sounds like a bad idea . there
> are plenty of usecases where libevdev_set_fd() is called but timestamps in
> events just don't matter. So we may need need some more negotiation between
> the library user, libevdev and the kernel.

I also don't see any strict dependency at all: the binary data format
has not changed, and I agree we absolutely should not break running
a newly built library on old kernels.

We can also safely assume that any user space that is actually built
for 64-bit time_t is also running on a recent enough kernel,
as today's kernels do not support 64-bit time_t yet.

Arnd
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-28 Thread Arnd Bergmann 
On Monday, October 17, 2016 8:27:32 PM CEST Deepa Dinamani wrote:
> @@ -55,24 +60,24 @@ struct ff_effect_compat {
>  
>  static inline size_t input_event_size(void)
>  {
> -   return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
> -   sizeof(struct input_event_compat) : sizeof(struct 
> input_event);
> +   return in_compat_syscall() ? sizeof(struct raw_input_event_compat) :
> +sizeof(struct raw_input_event);
>  }

I think the COMPAT_USE_64BIT_TIME check has to stay here,
it's needed for x32 mode on x86-64.

Arnd
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-28 Thread Arnd Bergmann 
On Thursday, October 27, 2016 4:12:54 PM CEST Dmitry Torokhov wrote:
> On Thu, Oct 27, 2016 at 03:25:43PM -0700, Deepa Dinamani wrote:
> > > If users are forced to update to adapt to the new event format, should
> > > we consider more radical changes? For example, does it make sense to
> > > send timestamp on every event? Maybe we should only send it once per
> > > event packet (between EV_SYN/SYN_REPORT)? What granularity do we need?
> > > Is there anything else in current protocol that we'd like to change?
> > 
> > I did see the thread with Pingbo's patches where you had a similar comment.
> > 
> > I see my series as decoupling the kernel input event format from the
> > userspace format.
> > The formats also are really the same still.
> > Could this be considered the first step towards changing the protocol?
> 
> I really do not see the point. I think we agree that the current
> protocol is not working past 2038 and it does not seem we can fix it
> transparently for the user. So we need to define new protocol and let
> kernel and clients negotiate which one is used.

This work is not primarily about fixing the protocol to work beyond
2038 (although as a side-effect it will work until 2106). The
main intention here is to not break existing applications when
they get recompiled against a C library that defines time_t as
64-bit.

> I am not concerned about in-kernel representation much as it does not
> get stored anywhere so we can adjust it as needed without too much
> effort.
>
> > The protocol changes might need new interfaces to be defined between 
> > libraries.
> > And, could end up being a substantial change.
> > Would a step by step approach make sense?
> 
> It would depend largely on the scope.

I think we should do those two things completely independently.
We need to do something now to preserve the current interfaces
for the glibc changes that are coming soon [1], and Deepa's
patches do that (though I now realize the changelog doesn't
mention the requirement).

An overhaul of the input_event handling with a new modern
but incompatible format may or may not be a good idea, but
this should be decided independently.

Arnd

[1] https://sourceware.org/glibc/wiki/Y2038ProofnessDesign
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-27 Thread Peter Hutterer 
On Thu, Oct 27, 2016 at 03:24:55PM -0700, Deepa Dinamani wrote:
> On Wed, Oct 26, 2016 at 7:56 PM, Peter Hutterer
>  wrote:
> > On Mon, Oct 17, 2016 at 08:27:32PM -0700, Deepa Dinamani wrote:
> >> struct timeval is not y2038 safe.
> >> All usage of timeval in the kernel will be replaced by
> >> y2038 safe structures.
> >>
> >> struct input_event maintains time for each input event.
> >> Real time timestamps are not ideal for input as this
> >> time can go backwards as noted in the patch a80b83b7b8
> >> by John Stultz. Hence, having the input_event.time fields
> >> only big enough for monotonic and boot times are
> >> sufficient.
> >>
> >> Leave the original input_event as is. This is to maintain
> >> backward compatibility with existing userspace interfaces
> >> that use input_event.
> >> Introduce a new replacement struct raw_input_event.
> >
> > general comment here - please don't name it "raw_input_event".
> > First, when you grep for input_event you want the new ones to show up too,
> > so a struct input_event_raw would be better here. That also has better
> > namespacing in general. Second though: the event isn't any more "raw" than
> > the previous we had.
> >
> > I can't think of anything better than struct input_event_v2 though.
> 
> The general idea was to leave the original struct input_event as a
> common interface for userspace (as it cannot be deleted).
> So reading raw data unformatted by the userspace will have the new
> struct raw_input_event format.
> This was the reason for the "raw" in the name.
> 
> struct input_event_v2 is fine too, if this is more preferred.
> 
> >> This replaces timeval with struct input_timeval. This structure
> >> maintains time in __kernel_ulong_t or compat_ulong_t to allow
> >> for architectures to override types as in the case of x32.
> >>
> >> The change requires any userspace utilities reading or writing
> >> from event nodes to update their reading format to match
> >> raw_input_event. The changes to the popular libraries will be
> >> posted along with the kernel changes.
> >> The driver version is also updated to reflect the change in
> >> event format.
> >
> > Doesn't this break *all* of userspace then? I don't see anything to
> > negotiate the type of input event the kernel gives me. And nothing right now
> > checks for EVDEV_VERSION, so they all just assume it's a struct
> > input_event. Best case, if the available events aren't a multiple of
> > sizeof(struct input_event) userspace will bomb out, but unless that happens,
> > everyone will just happily read old-style events.
> >
> > So we need some negotiation what is acceptable. Which also needs to address
> > the race conditions we're going to get when events start coming in before
> > the client has announced that it supports the new-style events.
> 
> No, this does not break any userspace right now.
> Both struct input_event and struct raw_input_event are exactly the same today.

oh, right, the ABI is the same. I see that now, thanks.

> This will be the case until a 2038-safe glibc is used with a 64 bit time_t 
> flag.
> 
> So these are the scenarios:
> 1. old kernel driver + new userspace
>   -- should still be ok until 2038. Version checks could help discover these
> 2. new kernel driver + old userspace (without recompiled with new 2038 gblic)
>   -- works because the format is really the same.
> 
> The patch I posted to libevdev checks this driver version.

btw, where did you post the libevdev patch? I haven't seen it anywhere I'm
subscribed to.

> And, hence any library that results in a call to libevdev_set_fd()
> will fail if it is not this updated driver.

without having seen the libevdev patch - that sounds like a bad idea . there
are plenty of usecases where libevdev_set_fd() is called but timestamps in
events just don't matter. So we may need need some more negotiation between
the library user, libevdev and the kernel.

Cheers,
   Peter

> We could just do a similar check in every library also.
> I think the latter would be better.
> 
> So, the kernel patches can go in as a no-op right now and then I can
> add version checks to respective user space libraries.

___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-27 Thread Deepa Dinamani 
>> struct timeval is not y2038 safe.
>> All usage of timeval in the kernel will be replaced by
>> y2038 safe structures.
>>
>> struct input_event maintains time for each input event.
>> Real time timestamps are not ideal for input as this
>> time can go backwards as noted in the patch a80b83b7b8
>> by John Stultz. Hence, having the input_event.time fields
>> only big enough for monotonic and boot times are
>> sufficient.
>>
>> Leave the original input_event as is. This is to maintain
>> backward compatibility with existing userspace interfaces
>> that use input_event.
>> Introduce a new replacement struct raw_input_event.
>> This replaces timeval with struct input_timeval. This structure
>> maintains time in __kernel_ulong_t or compat_ulong_t to allow
>> for architectures to override types as in the case of x32.
>>
>> The change requires any userspace utilities reading or writing
>> from event nodes to update their reading format to match
>> raw_input_event. The changes to the popular libraries will be
>> posted along with the kernel changes.
>> The driver version is also updated to reflect the change in
>> event format.
>
> If users are forced to update to adapt to the new event format, should
> we consider more radical changes? For example, does it make sense to
> send timestamp on every event? Maybe we should only send it once per
> event packet (between EV_SYN/SYN_REPORT)? What granularity do we need?
> Is there anything else in current protocol that we'd like to change?

I did see the thread with Pingbo's patches where you had a similar comment.

I see my series as decoupling the kernel input event format from the
userspace format.
The formats also are really the same still.
Could this be considered the first step towards changing the protocol?

The protocol changes might need new interfaces to be defined between libraries.
And, could end up being a substantial change.
Would a step by step approach make sense?

-Deepa
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-27 Thread Deepa Dinamani 
On Wed, Oct 26, 2016 at 7:56 PM, Peter Hutterer
 wrote:
> On Mon, Oct 17, 2016 at 08:27:32PM -0700, Deepa Dinamani wrote:
>> struct timeval is not y2038 safe.
>> All usage of timeval in the kernel will be replaced by
>> y2038 safe structures.
>>
>> struct input_event maintains time for each input event.
>> Real time timestamps are not ideal for input as this
>> time can go backwards as noted in the patch a80b83b7b8
>> by John Stultz. Hence, having the input_event.time fields
>> only big enough for monotonic and boot times are
>> sufficient.
>>
>> Leave the original input_event as is. This is to maintain
>> backward compatibility with existing userspace interfaces
>> that use input_event.
>> Introduce a new replacement struct raw_input_event.
>
> general comment here - please don't name it "raw_input_event".
> First, when you grep for input_event you want the new ones to show up too,
> so a struct input_event_raw would be better here. That also has better
> namespacing in general. Second though: the event isn't any more "raw" than
> the previous we had.
>
> I can't think of anything better than struct input_event_v2 though.

The general idea was to leave the original struct input_event as a
common interface for userspace (as it cannot be deleted).
So reading raw data unformatted by the userspace will have the new
struct raw_input_event format.
This was the reason for the "raw" in the name.

struct input_event_v2 is fine too, if this is more preferred.

>> This replaces timeval with struct input_timeval. This structure
>> maintains time in __kernel_ulong_t or compat_ulong_t to allow
>> for architectures to override types as in the case of x32.
>>
>> The change requires any userspace utilities reading or writing
>> from event nodes to update their reading format to match
>> raw_input_event. The changes to the popular libraries will be
>> posted along with the kernel changes.
>> The driver version is also updated to reflect the change in
>> event format.
>
> Doesn't this break *all* of userspace then? I don't see anything to
> negotiate the type of input event the kernel gives me. And nothing right now
> checks for EVDEV_VERSION, so they all just assume it's a struct
> input_event. Best case, if the available events aren't a multiple of
> sizeof(struct input_event) userspace will bomb out, but unless that happens,
> everyone will just happily read old-style events.
>
> So we need some negotiation what is acceptable. Which also needs to address
> the race conditions we're going to get when events start coming in before
> the client has announced that it supports the new-style events.

No, this does not break any userspace right now.
Both struct input_event and struct raw_input_event are exactly the same today.
This will be the case until a 2038-safe glibc is used with a 64 bit time_t flag.

So these are the scenarios:
1. old kernel driver + new userspace
  -- should still be ok until 2038. Version checks could help discover these
2. new kernel driver + old userspace (without recompiled with new 2038 gblic)
  -- works because the format is really the same.

The patch I posted to libevdev checks this driver version.
And, hence any library that results in a call to libevdev_set_fd()
will fail if it is not this updated driver.
We could just do a similar check in every library also.
I think the latter would be better.

So, the kernel patches can go in as a no-op right now and then I can
add version checks to respective user space libraries.

-Deepa
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-26 Thread Dmitry Torokhov 
Hi Deepa,

On Mon, Oct 17, 2016 at 08:27:32PM -0700, Deepa Dinamani wrote:
> struct timeval is not y2038 safe.
> All usage of timeval in the kernel will be replaced by
> y2038 safe structures.
> 
> struct input_event maintains time for each input event.
> Real time timestamps are not ideal for input as this
> time can go backwards as noted in the patch a80b83b7b8
> by John Stultz. Hence, having the input_event.time fields
> only big enough for monotonic and boot times are
> sufficient.
> 
> Leave the original input_event as is. This is to maintain
> backward compatibility with existing userspace interfaces
> that use input_event.
> Introduce a new replacement struct raw_input_event.
> This replaces timeval with struct input_timeval. This structure
> maintains time in __kernel_ulong_t or compat_ulong_t to allow
> for architectures to override types as in the case of x32.
> 
> The change requires any userspace utilities reading or writing
> from event nodes to update their reading format to match
> raw_input_event. The changes to the popular libraries will be
> posted along with the kernel changes.
> The driver version is also updated to reflect the change in
> event format.

If users are forced to update to adapt to the new event format, should
we consider more radical changes? For example, does it make sense to
send timestamp on every event? Maybe we should only send it once per
event packet (between EV_SYN/SYN_REPORT)? What granularity do we need?
Is there anything else in current protocol that we'd like to change?

Thanks.

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

[Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-17 Thread Deepa Dinamani 
struct timeval is not y2038 safe.
All usage of timeval in the kernel will be replaced by
y2038 safe structures.

struct input_event maintains time for each input event.
Real time timestamps are not ideal for input as this
time can go backwards as noted in the patch a80b83b7b8
by John Stultz. Hence, having the input_event.time fields
only big enough for monotonic and boot times are
sufficient.

Leave the original input_event as is. This is to maintain
backward compatibility with existing userspace interfaces
that use input_event.
Introduce a new replacement struct raw_input_event.
This replaces timeval with struct input_timeval. This structure
maintains time in __kernel_ulong_t or compat_ulong_t to allow
for architectures to override types as in the case of x32.

The change requires any userspace utilities reading or writing
from event nodes to update their reading format to match
raw_input_event. The changes to the popular libraries will be
posted along with the kernel changes.
The driver version is also updated to reflect the change in
event format.

Suggested-by: Arnd Bergmann 
Signed-off-by: Deepa Dinamani 
Reviewed-by: Arnd Bergmann 
---
 drivers/input/evdev.c| 20 +--
 drivers/input/input-compat.c | 29 ++-
 drivers/input/input-compat.h | 19 +++---
 drivers/input/misc/uinput.c  |  6 +++---
 include/linux/uinput.h   |  2 +-
 include/uapi/linux/input.h   | 47 
 6 files changed, 88 insertions(+), 35 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index b4e3171..459e3ba 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -60,7 +60,7 @@ struct evdev_client {
bool revoked;
unsigned long *evmasks[EV_CNT];
unsigned int bufsize;
-   struct input_event buffer[];
+   struct raw_input_event buffer[];
 };
 
 static size_t evdev_get_mask_cnt(unsigned int type)
@@ -113,7 +113,7 @@ static void __evdev_flush_queue(struct evdev_client 
*client, unsigned int type)
unsigned int i, head, num;
unsigned int mask = client->bufsize - 1;
bool is_report;
-   struct input_event *ev;
+   struct raw_input_event *ev;
 
BUG_ON(type == EV_SYN);
 
@@ -155,7 +155,7 @@ static void __evdev_flush_queue(struct evdev_client 
*client, unsigned int type)
 
 static void __evdev_queue_syn_dropped(struct evdev_client *client)
 {
-   struct input_event ev;
+   struct raw_input_event ev;
struct timespec64 ts;
 
switch (client->clk_type) {
@@ -236,7 +236,7 @@ static int evdev_set_clk_type(struct evdev_client *client, 
unsigned int clkid)
 }
 
 static void __pass_event(struct evdev_client *client,
-const struct input_event *event)
+const struct raw_input_event *event)
 {
client->buffer[client->head++] = *event;
client->head &= client->bufsize - 1;
@@ -268,7 +268,7 @@ static void evdev_pass_values(struct evdev_client *client,
 {
struct evdev *evdev = client->evdev;
const struct input_value *v;
-   struct input_event event;
+   struct raw_input_event event;
struct timespec64 ts;
bool wakeup = false;
 
@@ -507,7 +507,7 @@ static int evdev_open(struct inode *inode, struct file 
*file)
struct evdev *evdev = container_of(inode->i_cdev, struct evdev, cdev);
unsigned int bufsize = evdev_compute_buffer_size(evdev->handle.dev);
unsigned int size = sizeof(struct evdev_client) +
-   bufsize * sizeof(struct input_event);
+   bufsize * sizeof(struct raw_input_event);
struct evdev_client *client;
int error;
 
@@ -542,7 +542,7 @@ static ssize_t evdev_write(struct file *file, const char 
__user *buffer,
 {
struct evdev_client *client = file->private_data;
struct evdev *evdev = client->evdev;
-   struct input_event event;
+   struct raw_input_event event;
int retval = 0;
 
if (count != 0 && count < input_event_size())
@@ -575,7 +575,7 @@ static ssize_t evdev_write(struct file *file, const char 
__user *buffer,
 }
 
 static int evdev_fetch_next_event(struct evdev_client *client,
- struct input_event *event)
+ struct raw_input_event *event)
 {
int have_event;
 
@@ -597,7 +597,7 @@ static ssize_t evdev_read(struct file *file, char __user 
*buffer,
 {
struct evdev_client *client = file->private_data;
struct evdev *evdev = client->evdev;
-   struct input_event event;
+   struct raw_input_event event;
size_t read = 0;
int error;
 
@@ -1083,7 +1083,7 @@ static long evdev_do_ioctl(struct file *file, unsigned 
int cmd,
switch (cmd) {
 
case EVIOCGVERSION:
-   return put_user(EV_VERSION, ip);
+