Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/kern Undo previous, in the name of "defined" behaviour, ...



details:   https://anonhg.NetBSD.org/src/rev/3ae90da7bd86
branches:  trunk
changeset: 449547:3ae90da7bd86
user:      kre <kre%NetBSD.org@localhost>
date:      Sun Mar 10 13:44:49 2019 +0000

description:
Undo previous, in the name of "defined" behaviour, it breaks things.

This is all explained in the comment at the head of the file:

 * Some of the "math" in here is a bit tricky.  We have to beware of
 * wrapping ints.
 *
 * [...] but c->c_time can
 * be positive or negative so comparing it with anything is dangerous.

In particular, "if (c->c_time > ticks)" is simply wrong.

 * The only way we can use the c->c_time value in any predictable way is
 * when we calculate how far in the future `to' will timeout - "c->c_time
 * - c->c_cpu->cc_ticks".  The result will always be positive for future
 * timeouts and 0 or negative for due timeouts.

Go back to the old way.   But write the calculation of delta slightly
differently which will hopefully appease KUBsan.   Perhaps.  In any
case, this code works on any system that NetBSD has any hope of ever
running on, whatever the C standards say is "defined" behaviour.

diffstat:

 sys/kern/kern_timeout.c |  10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diffs (34 lines):

diff -r 8f39b20939d2 -r 3ae90da7bd86 sys/kern/kern_timeout.c
--- a/sys/kern/kern_timeout.c   Sun Mar 10 13:24:50 2019 +0000
+++ b/sys/kern/kern_timeout.c   Sun Mar 10 13:44:49 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_timeout.c,v 1.55 2018/07/08 14:42:52 kamil Exp $  */
+/*     $NetBSD: kern_timeout.c,v 1.56 2019/03/10 13:44:49 kre Exp $    */
 
 /*-
  * Copyright (c) 2003, 2006, 2007, 2008, 2009 The NetBSD Foundation, Inc.
@@ -59,7 +59,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_timeout.c,v 1.55 2018/07/08 14:42:52 kamil Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_timeout.c,v 1.56 2019/03/10 13:44:49 kre Exp $");
 
 /*
  * Timeouts are kept in a hierarchical timing wheel.  The c_time is the
@@ -717,12 +717,12 @@
 
                /* If due run it, otherwise insert it into the right bucket. */
                ticks = cc->cc_ticks;
-               if (c->c_time > ticks) {
-                       delta = c->c_time - ticks;
+               delta = (int)((unsigned)c->c_time - (unsigned)ticks);
+               if (delta > 0) {
                        CIRCQ_INSERT(&c->c_list, BUCKET(cc, delta, c->c_time));
                        continue;
                }
-               if (c->c_time < ticks)
+               if (delta < 0)
                        cc->cc_ev_late.ev_count++;
 
                c->c_flags = (c->c_flags & ~CALLOUT_PENDING) |



Home | Main Index | Thread Index | Old Index