From 4f76b6ba6ade149ced05c8b24b569bf9fac78c60 Mon Sep 17 00:00:00 2001 From: Alan Ngo Date: Tue, 9 Jun 2026 17:32:58 -0700 Subject: [PATCH 1/4] remove magic numbers from echo driver --- general/echo/kmdf/driver/DriverSync/src/device.rs | 5 ++++- general/echo/kmdf/driver/DriverSync/src/driver.rs | 4 ++-- general/echo/kmdf/driver/DriverSync/src/queue.rs | 6 ++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/general/echo/kmdf/driver/DriverSync/src/device.rs b/general/echo/kmdf/driver/DriverSync/src/device.rs index d452af1..4d8fd1d 100644 --- a/general/echo/kmdf/driver/DriverSync/src/device.rs +++ b/general/echo/kmdf/driver/DriverSync/src/device.rs @@ -30,6 +30,9 @@ use crate::{ WDF_REQUEST_CONTEXT_TYPE_INFO, }; +/// 100ms relative time (in nanoseconds units) +const WDF_REL_TIMEOUT_IN_MS: i64 = -(100) * (10000); + /// Worker routine called to create a device and its software resources. /// /// # Arguments: @@ -162,7 +165,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 = -(100) * (10000); + let due_time: i64 = WDF_REL_TIMEOUT_IN_MS; let _ = unsafe { (*queue_context).timer.start(due_time) }; diff --git a/general/echo/kmdf/driver/DriverSync/src/driver.rs b/general/echo/kmdf/driver/DriverSync/src/driver.rs index 93b9664..bf03985 100644 --- a/general/echo/kmdf/driver/DriverSync/src/driver.rs +++ b/general/echo/kmdf/driver/DriverSync/src/driver.rs @@ -177,9 +177,9 @@ fn echo_print_driver_version() -> NTSTATUS { call_unsafe_wdf_function_binding!(WdfDriverIsVersionAvailable, driver, &raw mut ver) } > 0 { - println!("Yes, framework version is 1.0"); + println!("Yes, framework version is {}.{}", ver.MajorVersion, ver.MinorVersion); } else { - println!("No, framework version is not 1.0"); + println!("No, framework version is not {}.{}", ver.MajorVersion, ver.MinorVersion); } STATUS_SUCCESS diff --git a/general/echo/kmdf/driver/DriverSync/src/queue.rs b/general/echo/kmdf/driver/DriverSync/src/queue.rs index 3780c95..e99e3a7 100644 --- a/general/echo/kmdf/driver/DriverSync/src/queue.rs +++ b/general/echo/kmdf/driver/DriverSync/src/queue.rs @@ -50,6 +50,9 @@ const MAX_WRITE_LENGTH: usize = 1024 * 40; /// Set timer period in ms const TIMER_PERIOD: u32 = 1000 * 10; +/// Non-zero char literal (of one to four chars) for pool tag used in ExAllocatePool2 +const MEMORY_TAG: u32 = u32::from_be_bytes(*b"sam1"); + /// This routine will interlock increment a value only if the current value /// is greater then the floor value. /// @@ -564,9 +567,8 @@ extern "C" fn echo_evt_io_write(queue: WDFQUEUE, request: WDFREQUEST, length: us (*queue_context).length = 0; } - // FIXME: Memory Tag (*queue_context).buffer = - ExAllocatePool2(POOL_FLAG_NON_PAGED, length as SIZE_T, 's' as u32); + ExAllocatePool2(POOL_FLAG_NON_PAGED, length as SIZE_T, MEMORY_TAG); if (*queue_context).buffer.is_null() { println!( "echo_evt_io_write Could not allocate {:?} byte buffer", From 756707fef9a387466dd00857068d98626c088096 Mon Sep 17 00:00:00 2001 From: Alan Ngo Date: Wed, 10 Jun 2026 11:46:37 -0700 Subject: [PATCH 2/4] more magic numbers removed --- .../echo/kmdf/driver/DriverSync/src/device.rs | 14 +++++++-- .../echo/kmdf/driver/DriverSync/src/queue.rs | 31 ++++++++++++++----- general/echo/kmdf/exe/src/main.rs | 21 +++++++++++-- 3 files changed, 54 insertions(+), 12 deletions(-) diff --git a/general/echo/kmdf/driver/DriverSync/src/device.rs b/general/echo/kmdf/driver/DriverSync/src/device.rs index 4d8fd1d..ba4decb 100644 --- a/general/echo/kmdf/driver/DriverSync/src/device.rs +++ b/general/echo/kmdf/driver/DriverSync/src/device.rs @@ -30,8 +30,18 @@ use crate::{ WDF_REQUEST_CONTEXT_TYPE_INFO, }; -/// 100ms relative time (in nanoseconds units) -const WDF_REL_TIMEOUT_IN_MS: i64 = -(100) * (10000); +/// Number of 100-nanosecond intervals in one millisecond. WDF relative times +/// are expressed in 100-nanosecond units, so this converts a millisecond delay +/// into the units WDF expects. +const RELATIVE_100_NS_INTERVALS_PER_MS: i64 = 10_000; + +/// Delay, in milliseconds, before the periodic timer first fires once the +/// device has started. +const START_TIMER_DUE_TIME_MS: i64 = 100; + +/// 100ms relative time (in 100-nanosecond units). The negative sign marks the +/// value as a relative (rather than absolute) timeout. +const WDF_REL_TIMEOUT_IN_MS: i64 = -START_TIMER_DUE_TIME_MS * RELATIVE_100_NS_INTERVALS_PER_MS; /// Worker routine called to create a device and its software resources. /// diff --git a/general/echo/kmdf/driver/DriverSync/src/queue.rs b/general/echo/kmdf/driver/DriverSync/src/queue.rs index e99e3a7..e6bc21b 100644 --- a/general/echo/kmdf/driver/DriverSync/src/queue.rs +++ b/general/echo/kmdf/driver/DriverSync/src/queue.rs @@ -47,12 +47,28 @@ use crate::{ /// Set max write length for testing const MAX_WRITE_LENGTH: usize = 1024 * 40; -/// Set timer period in ms -const TIMER_PERIOD: u32 = 1000 * 10; +/// Number of milliseconds in one second. +const MS_PER_SECOND: u32 = 1000; -/// Non-zero char literal (of one to four chars) for pool tag used in ExAllocatePool2 +/// Watchdog timer period, in seconds. +const TIMER_PERIOD_SECONDS: u32 = 10; + +/// Timer period in ms +const TIMER_PERIOD: u32 = TIMER_PERIOD_SECONDS * MS_PER_SECOND; + +/// Non-zero char literal (of one to four chars) for pool tag used in +/// `ExAllocatePool2` const MEMORY_TAG: u32 = u32::from_be_bytes(*b"sam1"); +/// 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. /// @@ -359,7 +375,8 @@ 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(1); + (*request_context).cancel_completion_ownership_count = + AtomicI32::new(INITIAL_CANCEL_OWNERSHIP_COUNT); } // Defer the completion to another thread from the timer dpc @@ -699,12 +716,12 @@ 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. - // 2 is the initial count we set when we initialized - // CancelCompletionOwnershipCount plus the call to + // TIMER_CLAIMED_OWNERSHIP_COUNT is the initial count we set when we + // initialized CancelCompletionOwnershipCount plus the call to // EchoIncrementRequestCancelOwnershipCount() (*request_context) .cancel_completion_ownership_count - .fetch_sub(2, Ordering::SeqCst); + .fetch_sub(TIMER_CLAIMED_OWNERSHIP_COUNT, Ordering::SeqCst); complete_request = true; } } diff --git a/general/echo/kmdf/exe/src/main.rs b/general/echo/kmdf/exe/src/main.rs index 13b0940..85f8335 100644 --- a/general/echo/kmdf/exe/src/main.rs +++ b/general/echo/kmdf/exe/src/main.rs @@ -70,6 +70,16 @@ static READER_TYPE: u32 = 1; static WRITER_TYPE: u32 = 2; static NUM_ASYNCH_IO: usize = 100; static BUFFER_SIZE: usize = 40 * 1024; +/// Transfer length, in bytes, for the first synchronous write/read test. +static SYNC_TEST_SMALL_LENGTH: u32 = 512; +/// Transfer length, in bytes, for the second synchronous write/read test. +static SYNC_TEST_LARGE_LENGTH: u32 = 30 * 1024; +/// Completion key associated with the device handle on the I/O completion port. +static COMPLETION_PORT_KEY: usize = 1; +/// Number of concurrent threads allowed to run for the I/O completion port. +/// Zero lets the system allow as many concurrent threads as there are +/// processors. +static COMPLETION_PORT_CONCURRENT_THREADS: u32 = 0; fn main() -> Result<(), Box> { let argument_vector: Vec = env::args().collect(); @@ -155,9 +165,9 @@ Exit the app anytime by pressing Ctrl-C h.join().unwrap().unwrap(); } else { - perform_write_read_test(h_device, 512)?; + perform_write_read_test(h_device, SYNC_TEST_SMALL_LENGTH)?; - perform_write_read_test(h_device, 30 * 1024)?; + perform_write_read_test(h_device, SYNC_TEST_LARGE_LENGTH)?; } Ok(()) @@ -334,7 +344,12 @@ fn async_io_work(io_type: u32) -> Result<(), Box> { // Call Win32 API FFI CreateIoCompletionPort to get handle for completing async // requests unsafe { - h_completion_port = CreateIoCompletionPort(h_device, std::ptr::null_mut(), 1, 0); + h_completion_port = CreateIoCompletionPort( + h_device, + std::ptr::null_mut(), + COMPLETION_PORT_KEY, + COMPLETION_PORT_CONCURRENT_THREADS, + ); } if h_completion_port.is_null() { From 589c93328c56f2adc4a993b7ec0775257b7ebc65 Mon Sep 17 00:00:00 2001 From: Alan Ngo Date: Wed, 10 Jun 2026 11:53:41 -0700 Subject: [PATCH 3/4] formatting --- general/echo/kmdf/driver/DriverSync/src/driver.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/general/echo/kmdf/driver/DriverSync/src/driver.rs b/general/echo/kmdf/driver/DriverSync/src/driver.rs index bf03985..1a21121 100644 --- a/general/echo/kmdf/driver/DriverSync/src/driver.rs +++ b/general/echo/kmdf/driver/DriverSync/src/driver.rs @@ -177,9 +177,15 @@ fn echo_print_driver_version() -> NTSTATUS { call_unsafe_wdf_function_binding!(WdfDriverIsVersionAvailable, driver, &raw mut ver) } > 0 { - println!("Yes, framework version is {}.{}", ver.MajorVersion, ver.MinorVersion); + println!( + "Yes, framework version is {}.{}", + ver.MajorVersion, ver.MinorVersion + ); } else { - println!("No, framework version is not {}.{}", ver.MajorVersion, ver.MinorVersion); + println!( + "No, framework version is not {}.{}", + ver.MajorVersion, ver.MinorVersion + ); } STATUS_SUCCESS From c55833dbe78518f31eacfd13e0c34482b6f08958 Mon Sep 17 00:00:00 2001 From: Alan Ngo Date: Wed, 10 Jun 2026 12:10:15 -0700 Subject: [PATCH 4/4] decompose magic numbers in a const --- general/echo/kmdf/driver/DriverSync/src/queue.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/general/echo/kmdf/driver/DriverSync/src/queue.rs b/general/echo/kmdf/driver/DriverSync/src/queue.rs index e6bc21b..9e47c82 100644 --- a/general/echo/kmdf/driver/DriverSync/src/queue.rs +++ b/general/echo/kmdf/driver/DriverSync/src/queue.rs @@ -44,8 +44,11 @@ use crate::{ WDF_TIMER_CONFIG_SIZE, }; -/// Set max write length for testing -const MAX_WRITE_LENGTH: usize = 1024 * 40; +/// Number of bytes in one kilobyte. +const BYTES_PER_KB: usize = 1024; + +/// Max write length, in bytes, for testing +const MAX_WRITE_LENGTH: usize = 40 * BYTES_PER_KB; /// Number of milliseconds in one second. const MS_PER_SECOND: u32 = 1000;