From a7f0f62fc064d21a3552570d4df9d35133baad13 Mon Sep 17 00:00:00 2001 From: wpaul Date: Fri, 30 Apr 2004 20:51:55 +0000 Subject: Small timer cleanups: - Use the dh_inserted member of the dispatch header in the Windows timer structure to indicate that the timer has been "inserted into the timer queue" (i.e. armed via timeout()). Use this as the value to return to the caller in KeCancelTimer(). Previously, I was using callout_pending(), but you can't use that with timeout()/untimeout() without creating a potential race condition. - Make ntoskrnl_init_timer() just a wrapper around ntoskrnl_init_timer_ex() (reduces some code duplication). - Drop Giant when entering if_ndis.c:ndis_tick() and subr_ntorkrnl.c:ntoskrnl_timercall(). At the moment, I'm forced to use system callwheel via timeout()/untimeout() to handle timers rather than the callout API (struct callout is too big to fit inside the Windows struct KTIMER, so I'm kind of hosed). Unfortunately, all the callouts in the callwhere are not marked as MPSAFE, so when one of them fires, it implicitly acquires Giant before invoking the callback routine (and releases it when it returns). I don't need to hold Giant, but there's no way to stop the callout code from acquiring it as long as I'm using timeout()/untimeout(), so for now we cheat by just dropping Giant right away (and re-acquiring it right before the routine returns so keep the callout code happy). At some point, I will need to solve this better, but for now this should be a suitable workaround. --- sys/compat/ndis/subr_ntoskrnl.c | 43 ++++++++++++++++++----------------------- sys/dev/if_ndis/if_ndis.c | 6 ++++++ 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/sys/compat/ndis/subr_ntoskrnl.c b/sys/compat/ndis/subr_ntoskrnl.c index d833a73..fb6c06d 100644 --- a/sys/compat/ndis/subr_ntoskrnl.c +++ b/sys/compat/ndis/subr_ntoskrnl.c @@ -1640,7 +1640,11 @@ ntoskrnl_timercall(arg) ktimer *timer; struct timeval tv; - timer = arg; + mtx_unlock(&Giant); + + timer = arg; + + timer->k_header.dh_inserted = FALSE; /* * If this is a periodic timer, re-arm it @@ -1654,6 +1658,7 @@ ntoskrnl_timercall(arg) if (timer->k_period) { tv.tv_sec = 0; tv.tv_usec = timer->k_period * 1000; + timer->k_header.dh_inserted = TRUE; timer->k_handle = timeout(ntoskrnl_timercall, timer, tvtohz(&tv)); } @@ -1663,6 +1668,8 @@ ntoskrnl_timercall(arg) ntoskrnl_wakeup(&timer->k_header); + mtx_lock(&Giant); + return; } @@ -1673,11 +1680,7 @@ ntoskrnl_init_timer(timer) if (timer == NULL) return; - INIT_LIST_HEAD((&timer->k_header.dh_waitlisthead)); - timer->k_header.dh_sigstate = FALSE; - timer->k_header.dh_type = EVENT_TYPE_NOTIFY; - timer->k_header.dh_size = OTYPE_TIMER; - callout_handle_init(&timer->k_handle); + ntoskrnl_init_timer_ex(timer, EVENT_TYPE_NOTIFY); return; } @@ -1692,6 +1695,7 @@ ntoskrnl_init_timer_ex(timer, type) INIT_LIST_HEAD((&timer->k_header.dh_waitlisthead)); timer->k_header.dh_sigstate = FALSE; + timer->k_header.dh_inserted = FALSE; timer->k_header.dh_type = type; timer->k_header.dh_size = OTYPE_TIMER; callout_handle_init(&timer->k_handle); @@ -1775,9 +1779,9 @@ ntoskrnl_set_timer_ex(timer, duetime, period, dpc) if (timer == NULL) return(FALSE); - if (timer->k_handle.callout != NULL && - callout_pending(timer->k_handle.callout)) { + if (timer->k_header.dh_inserted == TRUE) { untimeout(ntoskrnl_timercall, timer, timer->k_handle); + timer->k_header.dh_inserted = FALSE; pending = TRUE; } else pending = FALSE; @@ -1802,6 +1806,7 @@ ntoskrnl_set_timer_ex(timer, duetime, period, dpc) } } + timer->k_header.dh_inserted = TRUE; timer->k_handle = timeout(ntoskrnl_timercall, timer, tvtohz(&tv)); return(pending); @@ -1825,13 +1830,14 @@ ntoskrnl_cancel_timer(timer) if (timer == NULL) return(FALSE); - if (timer->k_handle.callout != NULL && - callout_pending(timer->k_handle.callout)) + if (timer->k_header.dh_inserted == TRUE) { + untimeout(ntoskrnl_timercall, timer, timer->k_handle); + if (timer->k_dpc != NULL) + ntoskrnl_dequeue_dpc(timer->k_dpc); pending = TRUE; - else + } else pending = FALSE; - untimeout(ntoskrnl_timercall, timer, timer->k_handle); return(pending); } @@ -1840,18 +1846,7 @@ __stdcall uint8_t ntoskrnl_read_timer(timer) ktimer *timer; { - uint8_t pending; - - if (timer == NULL) - return(FALSE); - - if (timer->k_handle.callout != NULL && - callout_pending(timer->k_handle.callout)) - pending = TRUE; - else - pending = FALSE; - - return(pending); + return(timer->k_header.dh_sigstate); } __stdcall static void diff --git a/sys/dev/if_ndis/if_ndis.c b/sys/dev/if_ndis/if_ndis.c index 3741ba4f..7b1251f 100644 --- a/sys/dev/if_ndis/if_ndis.c +++ b/sys/dev/if_ndis/if_ndis.c @@ -1014,12 +1014,18 @@ ndis_tick(xsc) void *xsc; { struct ndis_softc *sc; + + mtx_unlock(&Giant); + sc = xsc; ndis_sched(ndis_ticktask, sc, NDIS_TASKQUEUE); sc->ndis_stat_ch = timeout(ndis_tick, sc, hz * sc->ndis_block.nmb_checkforhangsecs); + mtx_lock(&Giant); + + return; } static void -- cgit v1.1