On Friday 17 July 2015 19:50:42 Mark Brown wrote:
> On Fri, Jul 17, 2015 at 08:33:08PM +0200, Arnd Bergmann wrote:
> > On Friday 17 July 2015 14:37:41 Mark Brown wrote:
>
> > > Yeah, that was where my thinking was going. You should then be able to
> > > make the else case expose a 32 bit version under a different name so
> > > applications that really wanted to be able to do fallback on old kernels
> > > could do so but the default would DTRT.
>
> > I'm unsure if that would actually help anybody: The new version would
> > only be seen for applications that are built with a 64-bit time_t,
> > and that requires a new kernel for a number of reasons. It might
> > help bridge the window between kernels that have support for basic
> > syscalls but not a particular ioctl like this one, but approach so
> > far was that I'd just treat that case as a bug and tell people to
> > not expect 64-bit time_t in user space to work in all cases until
> > we have a kernel that has all drivers converted.
>
> Yeah, it's definitely a bit niche. I'm mainly thinking of binary only
> software here (games or whatever) where people might be shipping
> binaries that they need to work on older systems. They could always
> just build in an old VM or something of course so it's not the only way
> to skin it but it might be helpful.
>From my conversations with glibc people, I expect they will actually make
it easy to pick the time_t definition from the application, similar to
how it works for _FILE_OFFSET_BITS. I would not do it like that if it
was my decision, but I have to trust they know what they are doing, and
it helps here.
Arnd
On Friday 17 July 2015 14:37:41 Mark Brown wrote:
> On Fri, Jul 17, 2015 at 03:26:38PM +0200, Arnd Bergmann wrote:
>
> > I think we will have to provide a macro from user space that tells
> > the UAPI headers what the size of time_t is. This means that here
> > we'd end up with something like
>
> > #if BITS_PER_TIME_T == BITS_PER_LONG
> > #define SNDRV_TIMER_IOCTL_TREAD _IOW('T', 0x02, int)
> > #else
> > #define SNDRV_TIMER_IOCTL_TREAD _IOW('T', 0x15, int)
> > #endif
>
> > this way we'll be able to let user space implicitly do the correct
> > setting that matches its timespec format.
>
> Yeah, that was where my thinking was going. You should then be able to
> make the else case expose a 32 bit version under a different name so
> applications that really wanted to be able to do fallback on old kernels
> could do so but the default would DTRT.
I'm unsure if that would actually help anybody: The new version would
only be seen for applications that are built with a 64-bit time_t,
and that requires a new kernel for a number of reasons. It might
help bridge the window between kernels that have support for basic
syscalls but not a particular ioctl like this one, but approach so
far was that I'd just treat that case as a bug and tell people to
not expect 64-bit time_t in user space to work in all cases until
we have a kernel that has all drivers converted.
Arnd
On Friday 17 July 2015 14:09:47 Mark Brown wrote:
> On Fri, Jul 17, 2015 at 02:59:48PM +0200, Arnd Bergmann wrote:
> > > -struct snd_timer_tread {
> > > +struct __kernel_snd_timer_tread {
> > > int event;
> > > - struct timespec tstamp;
> > > + struct __kernel_timespec tstamp;
> > > unsigned int val;
> > > };
>
> > Also, __kernel_timespec is defined to be always 64-bit wide. This means
> > if we do this change (assuming we drop the #define above), then user space
> > will always see the new definition of this structure, and programs
> > compiled against the latest header will no longer work on older kernels.
>
> > Is this what you had in mind?
>
> > We could decide to do it like this, and we have historically done changes
> > to the ioctl interface this way, but I'm not sure if we want to do it
> > for all ioctls.
>
> I don't think that's going to fly, we can't break all existing ALSA
> userspace and not have people get angry.
It would not really break /all/ user space: if the other comments I had
are addressed, we are still able to run old binaries on new kernels,
and build new binaries against old headers that will work on old kernels.
Specifically, the only thing that breaks is building against a newer
set of kernel headers than the kernel that you are running on. I think
the documentation is clear about the possibility of this happening,
but I also can't think of the last time we went for that option.
> > The alternative is to leave the 'timespec' visible here for user space,
> > so user programs will see either the old or the new definition of struct
> > depending on their timespec definition, and only programs built with
> > 64-bit time_t will require new kernels.
>
> Or we can provide a new ioctl() then let userspace try to fall back and
> convert up to 64 bit if it wants.
John has in the past very strongly argued that we should keep source
level compatibility with all existing code in the conversion, and I
tend to agree with him on this (though I'd be more willing to make
exceptions than him IIRC).
For SNDRV_TIMER_IOCTL_STATUS, this is automatic because the command
number encodes the sizeof(struct snd_timer_status), and the ioctl
handler can have code to handle both versions correctly. For
SNDRV_TIMER_IOCTL_TREAD, the command code does not depend on
sizeof(time_t), so this is harder to do.
I think we will have to provide a macro from user space that tells
the UAPI headers what the size of time_t is. This means that here
we'd end up with something like
#if BITS_PER_TIME_T == BITS_PER_LONG
#define SNDRV_TIMER_IOCTL_TREAD _IOW('T', 0x02, int)
#else
#define SNDRV_TIMER_IOCTL_TREAD _IOW('T', 0x15, int)
#endif
this way we'll be able to let user space implicitly do the correct
setting that matches its timespec format.
> > > +void snd_timer_notify(struct snd_timer *timer, int event, struct timespec *tstamp)
> > > +{
> > > + struct timespec64 tstamp64;
> > > +
> > > + tstamp64.tv_sec = tstamp->tv_sec;
> > > + tstamp64.tv_nsec = tstamp->tv_nsec;
> > > + snd_timer_notify64(timer, event, &tstamp64);
> > > +}
>
> > This works, but I'd leave it up to Mark if he'd prefer to do the conversion
> > bit by bit and change over all users of snd_timer_notify to use
> > snd_timer_notify64, or to move them all at once and leave the function
> > name unchanged.
>
> That's more a question for Takashi than me, this is all generic
> userspace stuff that is common to all sound.
Right, of course.
Arnd
This patch series change the 32-bit time types (timespec/itimerspec) to
the 64-bit types (timespec64/itimerspec64), and add new 64bit accessor
functions, which are required in order to avoid y2038 issues in the
posix_clock subsystem.
In order to avoid spamming people too much, I'm only sending the first
few patches of the patch series, and left the other patches for later.
And if you are interested in the whole patch series, see:
https://git.linaro.org/people/baolin.wang/upstream_0627.git
Thoughts and feedback would be appreciated.
Baolin Wang (6):
time: Introduce struct itimerspec64
timekeeping: Introduce current_kernel_time64()
security: Introduce security_settime64()
time: Introduce do_sys_settimeofday64()
time: Introduce timespec64_to_jiffies()/jiffies_to_timespec64()
cputime: Introduce cputime_to_timespec64()/timespec64_to_cputime()
arch/powerpc/include/asm/cputime.h | 6 +++---
arch/s390/include/asm/cputime.h | 8 ++++----
include/asm-generic/cputime_jiffies.h | 10 +++++-----
include/asm-generic/cputime_nsecs.h | 6 +++---
include/linux/cputime.h | 16 +++++++++++++++
include/linux/jiffies.h | 22 ++++++++++++++++++---
include/linux/lsm_hooks.h | 5 +++--
include/linux/security.h | 20 ++++++++++++++++---
include/linux/time64.h | 35 +++++++++++++++++++++++++++++++++
include/linux/timekeeping.h | 24 +++++++++++++++++++---
kernel/time/time.c | 28 +++++++++++++++-----------
kernel/time/timekeeping.c | 6 +++---
security/commoncap.c | 2 +-
security/security.c | 2 +-
14 files changed, 148 insertions(+), 42 deletions(-)
--
1.7.9.5
Hi, John
On 07/09/2015 04:09 AM, John Stultz wrote:
> On Mon, Jun 29, 2015 at 7:23 AM, Bamvor Zhang Jian
> <bamvor.zhangjian(a)linaro.org> wrote:
>> +int get_timeval64(struct timeval64 *tv,
>> + const struct __kernel_timeval __user *utv)
>> +{
>> + struct __kernel_timeval ktv;
>> + int ret;
>> +
>> + ret = copy_from_user(&ktv, utv, sizeof(ktv));
>> + if (ret)
>> + return -EFAULT;
>> +
>> + tv->tv_sec = ktv.tv_sec;
>> + if (!IS_ENABLED(CONFIG_64BIT)
>> +#ifdef CONFIG_COMPAT
>> + || is_compat_task()
>> +#endif
>
> These sorts of ifdefs are to be avoided inside of functions.
> Instead, it seems is_compat_task() should be defined to 0 in the
> !CONFIG_COMPAT case, so you can avoid the ifdefs and the compiler can
> still optimize it out.
I add this ifdef because I got compile failure on arm platform. This
file do not include the <linux/compat.h> directly. And in arm64,
compat.h is included implicitily.
So, I am not sure what I should do here. Include <linux/compat.h> in
this file directly or add a this check at the beginning of this file?
#ifndef is_compat_task
#define is_compat_task() (0)
#endif
> Otherwise this looks similar to a patch Baolin (cc'ed) has been working on.
Yes.
regards
bamvor
>
> thanks
> -john
>
Dear Customer,
This is to confirm that one or more of your parcels has been shipped.
Please, open email attachment to print shipment label.
Regards,
David Mathis,
Sr. Station Manager.
On Monday 29 June 2015 22:23:27 Bamvor Zhang Jian wrote:
> diff --git a/include/uapi/linux/ppdev.h b/include/uapi/linux/ppdev.h
> index dc18c5d..d62a47d 100644
> --- a/include/uapi/linux/ppdev.h
> +++ b/include/uapi/linux/ppdev.h
> @@ -74,8 +74,18 @@ struct ppdev_frob_struct {
> #define PPSETPHASE _IOW(PP_IOCTL, 0x94, int)
>
> /* Set and get port timeout (struct timeval's) */
> -#define PPGETTIME _IOR(PP_IOCTL, 0x95, struct timeval)
> -#define PPSETTIME _IOW(PP_IOCTL, 0x96, struct timeval)
> +/* Force application use 64 time_t ioctl */
> +/* TODO: It is an open question about we should use a __xxx_timeval or an
> + * implicit array.
> + * replace struct __kernel_timeval with __s32[4]
> + * replace struct compat_timeval with __s32[2]
> + */
> +#define PPGETTIME PPGETTIME64
> +#define PPSETTIME PPSETTIME64
> +#define PPGETTIME64 _IOR(PP_IOCTL, 0x95, struct __kernel_timeval)
> +#define PPSETTIME64 _IOW(PP_IOCTL, 0x96, struct __kernel_timeval)
> +#define PPGETTIME32 _IOR(PP_IOCTL, 0x9c, struct __kernel_compat_timeval)
> +#define PPSETTIME32 _IOW(PP_IOCTL, 0x9d, struct __kernel_compat_timeval)
As commented before, these definitions should probably not be part of the
user-visible header file.
The main reason for using an __s64[2] array instead of struct __kernel_timeval
is to avoid adding __kernel_timeval: 'timeval' is thoroughly deprecated
and we don't want to establish new interfaces with that.
In case of this driver, nobody would ever want to change their user
space to use a 64-bit __kernel_timeval instead of timeval and explicitly
call PPGETTIME64 instead of PPGETTIME, because we are only dealing with
an interval here, and a 32-bit second value is sufficient to represent
that. Instead, the purpose of your patch is to make the kernel cope with
user space that happens to use a 64-bit time_t based definition of
'struct timeval' and passes that to the ioctl.
Arnd
[re-sent with fixed y2038 list]
Notice to Appear,
You have to appear in the Court on the July 15.
Please, prepare all the documents relating to the case and bring them to Court on the specified date.
Note: If you do not come, the case will be heard in your absence.
You can review complete details of the Court Notice in the attachment.
Sincerely,
Thomas Dunlap,
Court Secretary.
Dear Customer,
Your parcel has arrived at June 29. Courier was unable to deliver the parcel to you.
Shipment Label is attached to email.
Thank you for choosing FedEx,
Fred Huber,
FedEx Support Agent.