- make naming and comments more clear

- revert unnecessary const's to literals w/improved comments

Author: Alan Ngo

general/echo/kmdf/driver/DriverSync/src/device.rs

@@ -148,18 +148,22 @@ pub fn echo_device_create(mut device_init: &mut WDFDEVICE_INIT) -> NTSTATUS {
 ///
 /// * `NTSTATUS` - Failures will result in the device stack being torn down.
 extern "C" fn echo_evt_device_self_managed_io_start(device: WDFDEVICE) -> NTSTATUS {
-    /// 100ms relative time (in 100-nanosecond units). The negative sign marks
-    /// the value as a relative (rather than absolute) timeout.
+    /// 100ms relative due time for a WDF timer, in system time units.
     #[allow(
         clippy::cast_possible_truncation,
         reason = "100ms in 100-nanosecond units is known to fit in i64"
     )]
-    const WDF_REL_TIMEOUT_100_MS: i64 = {
-        const UNITS: u128 = Duration::from_millis(100).as_nanos() / 100;
+    const WDF_TIMER_DUE_TIME_100_MS: i64 = {
+        // System time units are 100-nanosecond intervals.
+        const SYSTEM_TIME_UNITS: u128 = Duration::from_millis(100).as_nanos() / 100;
         const {
-            assert!(UNITS <= i64::MAX as u128, "1,000,000 should fit in i64");
+            assert!(
+                SYSTEM_TIME_UNITS <= i64::MAX as u128,
+                "SYSTEM_TIME_UNITS should fit in i64"
+            );
         };
-        -(UNITS as i64)
+        // Negative marks the value as a relative (rather than absolute) timeout.
+        -(SYSTEM_TIME_UNITS as i64)
     };
 
     // Restart the queue and the periodic timer. We stopped them before going
@@ -178,7 +182,7 @@ extern "C" fn echo_evt_device_self_managed_io_start(device: WDFDEVICE) -> NTSTAT
     // into low power state.
     unsafe { call_unsafe_wdf_function_binding!(WdfIoQueueStart, queue) };
 
-    let due_time: i64 = WDF_REL_TIMEOUT_100_MS;
+    let due_time: i64 = WDF_TIMER_DUE_TIME_100_MS;
 
     let _ = unsafe { (*queue_context).timer.start(due_time) };
 

general/echo/kmdf/driver/DriverSync/src/queue.rs

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // License: MIT OR Apache-2.0
 
-use core::{sync::atomic::Ordering, time::Duration};
+use core::sync::atomic::Ordering;
 
 use wdk::{nt_success, paged_code, println, wdf};
 use wdk_sys::{
@@ -44,15 +44,6 @@ use crate::{
     WDF_TIMER_CONFIG_SIZE,
 };
 
-/// Initial cancel/completion ownership count assigned to a new request. A
-/// claimant takes ownership by decrementing the count down to zero.
-const INITIAL_CANCEL_OWNERSHIP_COUNT: i32 = 1;
-
-/// Total ownership count held by the timer DPC once it has claimed completion
-/// of a request: the initial count plus the single increment it acquired via
-/// `echo_increment_request_cancel_ownership_count`.
-const TIMER_CLAIMED_OWNERSHIP_COUNT: i32 = INITIAL_CANCEL_OWNERSHIP_COUNT + 1;
-
 /// This routine will interlock increment a value only if the current value
 /// is greater then the floor value.
 ///
@@ -133,19 +124,6 @@ fn echo_interlocked_increment_gtzero(target: &AtomicI32) -> i32 {
 /// * `NTSTATUS`
 #[link_section = "PAGE"]
 pub unsafe fn echo_queue_initialize(device: WDFDEVICE) -> NTSTATUS {
-    /// Timer period of 10 seconds in ms
-    #[allow(
-        clippy::cast_possible_truncation,
-        reason = "10 seconds in millisecond units is known to fit in u32"
-    )]
-    const TIMER_PERIOD_10_S: u32 = {
-        const MILLIS: u128 = Duration::from_secs(10).as_millis();
-        const {
-            assert!(MILLIS <= u32::MAX as u128, "10,000 should fit in u32");
-        };
-        MILLIS as u32
-    };
-
     paged_code!();
 
     let mut queue = WDF_NO_HANDLE as WDFQUEUE;
@@ -222,7 +200,7 @@ pub unsafe fn echo_queue_initialize(device: WDFDEVICE) -> NTSTATUS {
     let mut timer_config = WDF_TIMER_CONFIG {
         Size: WDF_TIMER_CONFIG_SIZE,
         EvtTimerFunc: Some(echo_evt_timer_func),
-        Period: TIMER_PERIOD_10_S,
+        Period: 10_000, // 10 seconds, in milliseconds
         AutomaticSerialization: u8::from(true),
         TolerableDelay: 0,
         ..WDF_TIMER_CONFIG::default()
@@ -372,8 +350,7 @@ fn echo_set_current_request(request: WDFREQUEST, queue: WDFQUEUE) {
     // they will interlock decrement the count.  When the count reaches zero,
     // ownership has been acquired and the caller may complete the request.
     unsafe {
-        (*request_context).cancel_completion_ownership_count =
-            AtomicI32::new(INITIAL_CANCEL_OWNERSHIP_COUNT);
+        (*request_context).cancel_completion_ownership_count = AtomicI32::new(1);
     }
 
     // Defer the completion to another thread from the timer dpc
@@ -722,12 +699,11 @@ unsafe extern "C" fn echo_evt_timer_func(timer: WDFTIMER) {
             // currently racing with it), there is no need to use an interlocked
             // decrement to lower the cancel ownership count.
 
-            // TIMER_CLAIMED_OWNERSHIP_COUNT is the initial count we set when we
-            // initialized CancelCompletionOwnershipCount plus the call to
-            // EchoIncrementRequestCancelOwnershipCount()
+            // 2 = the initial ownership count (1) plus the one increment
+            // acquired via echo_increment_request_cancel_ownership_count.
             (*request_context)
                 .cancel_completion_ownership_count
-                .fetch_sub(TIMER_CLAIMED_OWNERSHIP_COUNT, Ordering::SeqCst);
+                .fetch_sub(2, Ordering::SeqCst);
             complete_request = true;
         }
     }