Messages in this thread Patch in this message | | | Date | Tue, 12 Jan 1999 01:20:39 +0100 (CET) | From | Andrea Arcangeli <> | Subject | [patch] Re: PATCH for 2.2-pre: poll timeout (was Re: Serious bug) |
| |
On Sun, 10 Jan 1999, Chip Salzenberg wrote:
> I agree with your analysis. I'm now running with the below patch. > (I decided to look into this because I use mutt. :-))
I just fixed and reported the bug ages ago and I think to have fixed it in a way that could be a bit more useful in the future.
Here it is my _old_ fix that is in my arca-patches from some days after schedule_timeout() went in the kernel. patch against 2.2.0-pre7:
Index: linux/include/linux/time.h diff -u linux/include/linux/time.h:1.1.1.2 linux/include/linux/time.h:1.1.1.1.2.17 --- linux/include/linux/time.h:1.1.1.2 Fri Nov 27 11:19:00 1998 +++ linux/include/linux/time.h Tue Dec 29 16:18:18 1998 @@ -13,43 +13,111 @@ #endif /* _STRUCT_TIMESPEC */ /* - * Change timeval to jiffies, trying to avoid the - * most obvious overflows.. + * I reimplmeneted the code in some big macro taking care of all possible + * overflows. I started hacking from the previous Linux code in this file. + * NOTE: The long macros in this file avoid _source_ code duplication + * and must be used only in the specific cases that will flollow. + * NOTE: The code `retval = xsec + sec' obviously can' t be exchanged with + * something like `retval = xsec + xxsec + sec'. + * Copright (C) 1998 Andrea Arcangeli * - * And some not so obvious. + * This code should to avoid all possible overflows... * - * Note that we don't want to return MAX_LONG, because - * for various timeout reasons we often end up having - * to wait "jiffies+1" in order to guarantee that we wait - * at _least_ "jiffies" - so "jiffies+1" had better still - * be positive. + * And some not so obvious. */ -#define MAX_JIFFY_OFFSET ((~0UL >> 1)-1) -static __inline__ unsigned long -timespec_to_jiffies(struct timespec *value) -{ - unsigned long sec = value->tv_sec; - long nsec = value->tv_nsec; +#define MAX_JIFFY_OFFSET (~0UL >> 1) - if (sec >= (MAX_JIFFY_OFFSET / HZ)) - return MAX_JIFFY_OFFSET; - nsec += 1000000000L / HZ - 1; - nsec /= 1000000000L / HZ; - return HZ * sec + nsec; +#define TIME_TSTRUCT_TO_JIFFIES(TIMEX, LITTLE_FIELD) \ + unsigned long sec = value->tv_sec, xsec = value->LITTLE_FIELD; \ + unsigned long xsec_tmp; \ + long retval; \ + \ + if (sec >= (MAX_JIFFY_OFFSET) / HZ) \ + return MAX_JIFFY_OFFSET; \ + sec *= HZ; \ + \ + xsec_tmp = xsec + (TIMEX+HZ-1)/HZ - 1; \ + if (xsec_tmp > xsec) \ + xsec = xsec_tmp; \ + xsec /= (TIMEX+HZ-1)/HZ; \ + if (xsec >= (MAX_JIFFY_OFFSET)) \ + return MAX_JIFFY_OFFSET; \ + \ + retval = xsec + sec; \ + if (retval < 0) \ + return MAX_JIFFY_OFFSET; \ + else if (!retval) \ + return value->tv_sec || value->LITTLE_FIELD; \ + \ + return retval; + +#define TIME_JIFFIES_TO_TSTRUCT(TIMEX, LITTLE_FIELD) \ + value->tv_sec = jiffies / HZ; \ + value->LITTLE_FIELD = (jiffies % HZ) * (TIMEX / HZ); + +/* + * change timespec to jiffies + */ +static __inline__ long +timespec_to_jiffies(const struct timespec *value) +{ + TIME_TSTRUCT_TO_JIFFIES(1000000000UL, tv_nsec); } static __inline__ void -jiffies_to_timespec(unsigned long jiffies, struct timespec *value) +jiffies_to_timespec(const unsigned long jiffies, struct timespec *value) { - value->tv_nsec = (jiffies % HZ) * (1000000000L / HZ); - value->tv_sec = jiffies / HZ; + TIME_JIFFIES_TO_TSTRUCT(1000000000L, tv_nsec); } - + struct timeval { time_t tv_sec; /* seconds */ suseconds_t tv_usec; /* microseconds */ }; + +static __inline__ long +timeval_to_jiffies(const struct timeval *value) +{ + TIME_TSTRUCT_TO_JIFFIES(1000000UL, tv_usec) +} + +static __inline__ void +jiffies_to_timeval(const unsigned long jiffies, struct timeval *value) +{ + TIME_JIFFIES_TO_TSTRUCT(1000000L, tv_usec); +} + +#define TIME_LITTLE_XSEC_TO_JIFFIES(XTIME) \ + if (xsec >= (MAX_JIFFY_OFFSET) / ((HZ+(XTIME-1))/XTIME)) \ + return MAX_JIFFY_OFFSET; \ + xsec = (xsec*HZ+(XTIME-1))/XTIME; + +#define TIME_BIG_XSEC_TO_JIFFIES(XTIME) \ + unsigned long xsec_tmp, xsec_orig = xsec; \ + xsec_tmp = xsec + (XTIME+HZ-1)/HZ - 1; \ + if (xsec_tmp > xsec) \ + xsec = xsec_tmp; \ + xsec = xsec / ((XTIME+HZ-1)/HZ); \ + \ + if ((signed) xsec < 0) \ + return MAX_JIFFY_OFFSET; \ + else if (!xsec) \ + return xsec_orig != 0; + +/* + * change msec to jiffies + */ +static __inline__ long +msec_to_jiffies(unsigned long xsec) +{ +#if HZ >= 1000 + TIME_LITTLE_XSEC_TO_JIFFIES(1000); +#else + TIME_BIG_XSEC_TO_JIFFIES(1000); +#endif + return xsec; +} struct timezone { int tz_minuteswest; /* minutes west of Greenwich */ Index: linux/kernel/itimer.c diff -u linux/kernel/itimer.c:1.1.1.2 linux/kernel/itimer.c:1.1.1.1.2.6 --- linux/kernel/itimer.c:1.1.1.2 Fri Nov 27 11:19:08 1998 +++ linux/kernel/itimer.c Fri Dec 18 23:55:17 1998 @@ -22,6 +22,7 @@ * alarm() uses itimers without checking, we have to use unsigned * arithmetic). */ +#if 0 /* use the time.h functions now -arca */ static unsigned long tvtojiffies(struct timeval *value) { unsigned long sec = (unsigned) value->tv_sec; @@ -33,13 +34,8 @@ usec /= 1000000 / HZ; return HZ*sec+usec; } +#endif -static void jiffiestotv(unsigned long jiffies, struct timeval *value) -{ - value->tv_usec = (jiffies % HZ) * (1000000 / HZ); - value->tv_sec = jiffies / HZ; -} - int do_getitimer(int which, struct itimerval *value) { register unsigned long val, interval; @@ -69,8 +65,8 @@ default: return(-EINVAL); } - jiffiestotv(val, &value->it_value); - jiffiestotv(interval, &value->it_interval); + jiffies_to_timeval(val, &value->it_value); + jiffies_to_timeval(interval, &value->it_interval); return 0; } @@ -109,8 +105,8 @@ register unsigned long i, j; int k; - i = tvtojiffies(&value->it_interval); - j = tvtojiffies(&value->it_value); + i = timeval_to_jiffies(&value->it_interval); + j = timeval_to_jiffies(&value->it_value); if (ovalue && (k = do_getitimer(which, ovalue)) < 0) return k; switch (which) { Index: linux/kernel/sched.c diff -u linux/kernel/sched.c:1.1.1.5 linux/kernel/sched.c:1.1.1.1.2.37 --- linux/kernel/sched.c:1.1.1.5 Thu Jan 7 12:21:34 1999 +++ linux/kernel/sched.c Thu Jan 7 12:57:23 1999 @@ -1805,7 +1806,7 @@ return 0; } - expire = timespec_to_jiffies(&t) + (t.tv_sec || t.tv_nsec); + expire = timespec_to_jiffies(&t); current->state = TASK_INTERRUPTIBLE; expire = schedule_timeout(expire); Index: linux/kernel/signal.c diff -u linux/kernel/signal.c:1.1.1.3 linux/kernel/signal.c:1.1.1.1.2.7 --- linux/kernel/signal.c:1.1.1.3 Sun Dec 20 16:31:08 1998 +++ linux/kernel/signal.c Sun Dec 20 16:51:31 1998 @@ -738,8 +738,7 @@ timeout = MAX_SCHEDULE_TIMEOUT; if (uts) - timeout = (timespec_to_jiffies(&ts) - + (ts.tv_sec || ts.tv_nsec)); + timeout = timespec_to_jiffies(&ts); current->state = TASK_INTERRUPTIBLE; timeout = schedule_timeout(timeout); Index: linux/fs/select.c diff -u linux/fs/select.c:1.1.1.3 linux/fs/select.c:1.1.1.1.2.8 --- linux/fs/select.c:1.1.1.3 Mon Jan 11 22:22:14 1999 +++ linux/fs/select.c Mon Jan 11 22:56:02 1999 @@ -17,7 +17,6 @@ #include <asm/uaccess.h> -#define ROUND_UP(x,y) (((x)+(y)-1)/(y)) #define DEFAULT_POLLMASK (POLLIN | POLLOUT | POLLRDNORM | POLLWRNORM) /* @@ -202,9 +201,6 @@ * Update: ERESTARTSYS breaks at least the xview clock binary, so * I'm trying ERESTARTNOHAND which restart only when you want to. */ -#define MAX_SELECT_SECONDS \ - ((unsigned long) (MAX_SCHEDULE_TIMEOUT / HZ)-1) - asmlinkage int sys_select(int n, fd_set *inp, fd_set *outp, fd_set *exp, struct timeval *tvp) { @@ -212,24 +208,21 @@ long timeout; int ret; - timeout = MAX_SCHEDULE_TIMEOUT; if (tvp) { - time_t sec, usec; + struct timeval ktvp; if ((ret = verify_area(VERIFY_READ, tvp, sizeof(*tvp))) - || (ret = __get_user(sec, &tvp->tv_sec)) - || (ret = __get_user(usec, &tvp->tv_usec))) + || (ret = __get_user(ktvp.tv_sec, &tvp->tv_sec)) + || (ret = __get_user(ktvp.tv_usec, &tvp->tv_usec))) goto out_nofds; ret = -EINVAL; - if (sec < 0 || usec < 0) + if (ktvp.tv_sec < 0 || ktvp.tv_usec < 0) goto out_nofds; - if ((unsigned long) sec < MAX_SELECT_SECONDS) { - timeout = ROUND_UP(usec, 1000000/HZ); - timeout += sec * (unsigned long) HZ; - } - } + timeout = timeval_to_jiffies(&ktvp); + } else + timeout = MAX_SCHEDULE_TIMEOUT; ret = -ENOMEM; fds = (fd_set_buffer *) __get_free_page(GFP_KERNEL); @@ -251,14 +244,10 @@ ret = do_select(n, fds, &timeout); if (tvp && !(current->personality & STICKY_TIMEOUTS)) { - time_t sec = 0, usec = 0; - if (timeout) { - sec = timeout / HZ; - usec = timeout % HZ; - usec *= (1000000/HZ); - } - put_user(sec, &tvp->tv_sec); - put_user(usec, &tvp->tv_usec); + struct timeval ktvp; + jiffies_to_timeval(timeout, &ktvp); + put_user(ktvp.tv_sec, &tvp->tv_sec); + put_user(ktvp.tv_usec, &tvp->tv_usec); } if (ret < 0) @@ -335,11 +324,11 @@ if (nfds > NR_OPEN) goto out; - if (timeout) { - /* Carefula about overflow in the intermediate values */ - if ((unsigned long) timeout < MAX_SCHEDULE_TIMEOUT / HZ) - timeout = (timeout*HZ+999)/1000+1; - else /* Negative or overflow */ + if (timeout) + { + if (timeout > 0) + timeout = msec_to_jiffies(timeout); + else timeout = MAX_SCHEDULE_TIMEOUT; }
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |