Hi, arnd
On 12/05/2015 12:44 AM, Arnd Bergmann wrote:
On Friday 04 December 2015 23:11:58 Bamvor Jian Zhang wrote:
diff --git a/drivers/char/lp.c b/drivers/char/lp.c index c4094c4..a207e0c 100644 --- a/drivers/char/lp.c +++ b/drivers/char/lp.c @@ -136,6 +136,14 @@ #include <asm/irq.h> #include <asm/uaccess.h>
+/*
- FIXME: It should be removed after COMPAT_USE_64BIT_TIME is accessible for
- 32bit architecture.
- */
+#ifndef COMPAT_USE_64BIT_TIME +#define COMPAT_USE_64BIT_TIME (0) +#endif /* COMPAT_USE_64BIT_TIME */
Unfortunately, I think this can't work: the kernel has no idea whether user space uses a 32-bit or 64-bit time_t, and we need to be able to support both kinds of user space at the same time.
What we have to do instead is change the definition of LPSETTIMEOUT, like
#if __BITS_PER_SIZE_T >= __BITS_PER_LONG #define LPSETTIMEOUT 0x060f /* set parport timeout */ #else #define LPSETTIMEOUT _IOW(0x06, 0x0f, struct timeval) #endif
We need to decide what the correct #if condition should be, but the idea is that all current user space remains unchanged, while user space built with a 64-bit time_t sees the modified definition of LPSETTIMEOUT and passes that into the kernel when using the larger structure.
I am trying to follow your suggestion. But I do not understand why `__BITS_PER_SIZE_T` is relative to sizeof(time_t). And I also tried to use `__BITS_PER_TIME_T` according to the following table. /* * userspace | kernel | `__BITS_PER_TIME_T` | `__BITS_PER_LONG` | `__BITS_PER_TIME_T` <= `__BITS_PER_LONG` * -----------|----------|---------------------|-------------------|------------------------------------------ * 32 | 32 | 32 | 32 | true * 32 | 32 | 64 | 32 | false * 32 | 64 | 32 | 32 | true * 32 | 64 | 64 | 32 | false * 64 | 64 | 64 | 64 | true */ #if __BITS_PER_TIME_T <= __BITS_PER_LONG #define LPSETTIMEOUT 0x060f /* set parport timeout */ #else #define LPSETTIMEOUT _IOW(0x06, 0x0f, struct timeval) #endif
As you know, there is no `__BITS_PER_TIME_T` as well. Finally, I thought I could make use of `__USE_TIME_BITS64` which mentioned in https://sourceware.org/glibc/wiki/Y2038ProofnessDesign
How about something like this?
#if defined(__USE_TIME_BITS64) && __BITS_PER_LONG == 32 #define LPSETTIMEOUT _IOW(0x06, 0x0f, struct timeval) #else #define LPSETTIMEOUT 0x060f /* set parport timeout */ #endif
Regards
Bamvor
/* if you have more than 8 printers, remember to increase LP_NO */ #define LP_NO 8
@@ -572,6 +580,22 @@ static int lp_release(struct inode * inode, struct file * file) return 0; }
+static int lp_set_timeout(unsigned int minor, s64 tv_sec, s64 tv_usec)
tv_usec does not need to be 64-bit here, it has to be below 1 million.
+{
- long to_jiffies;
- if ((tv_sec < 0) || (tv_usec < 0))
- return -EINVAL;
- to_jiffies = usecs_to_jiffies(tv_usec);
- to_jiffies += tv_sec * (long)HZ;
- if (to_jiffies <= 0)
- return -EINVAL;
- lp_table[minor].timeout = to_jiffies;
- return 0;
+}
I think tv_sec can also be shorter, as we ignore the upper bits after the multiplication with HZ.
Arnd