Re: Jiffy wraparound problems.

Andrea Arcangeli (andrea@e-mind.com)
Thu, 29 Apr 1999 16:37:53 +0200 (CEST)


On Wed, 28 Apr 1999, David Benson wrote:

>The 2.2 problems are somewhat conjectural, but I wrote a test
>kernel-module which sets `jiffies' to 0xf000000; this machine then
>exhibits the select bug.

Some time ago I rewrote all timeval/conversion functions. Then I stop
using my code because I trusted the stock-kernel functions was just OK.

I found an old patch I did against 2.2.0-pre7-test1. If you can try if
applys against 2.2.7 and if fix your problems...

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;
}

Andrea Arcangeli

-
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/