issue #127: Remove timerConfig.AutomaticSerialization = FALSE from KMDF echo sample

4 files changed, 34 insertions, 37 deletions

diff --git a/general/echo/kmdf/driver/AutoSync/queue.c b/general/echo/kmdf/driver/AutoSync/queue.c
index b821add2..d881c003 100644
--- a/general/echo/kmdf/driver/AutoSync/queue.c
+++ b/general/echo/kmdf/driver/AutoSync/queue.c
@@ -86,7 +86,7 @@ Return Value:
     // with the same lock.
     //
     queueAttributes.SynchronizationScope = WdfSynchronizationScopeQueue;
-    
+
     queueAttributes.EvtDestroyCallback = EchoEvtIoQueueContextDestroy;
 
     status = WdfIoQueueCreate(
@@ -155,12 +155,12 @@ Return Value:
     PAGED_CODE();
 
     //
-    // Create a WDFTIMER object
+    // Create a periodic timer.
+    //
+    // WDF_TIMER_CONFIG_INIT_PERIODIC sets AutomaticSerialization to TRUE by default.
     //
     WDF_TIMER_CONFIG_INIT_PERIODIC(&timerConfig, EchoEvtTimerFunc, Period);
 
-    timerConfig.AutomaticSerialization = FALSE;
-
     WDF_OBJECT_ATTRIBUTES_INIT(&timerAttributes);
     timerAttributes.ParentObject = Queue; // Synchronize with the I/O Queue
 
diff --git a/general/echo/kmdf/driver/DriverSync/device.c b/general/echo/kmdf/driver/DriverSync/device.c
index c9dbcbb4..23f684e9 100644
--- a/general/echo/kmdf/driver/DriverSync/device.c
+++ b/general/echo/kmdf/driver/DriverSync/device.c
@@ -77,17 +77,6 @@ Return Value:
 
     WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&attributes, DEVICE_CONTEXT);
 
-    //
-    // By not setting the synchronization scope and using the default, there is
-    // no locking between any of the callbacks in this driver.
-    //
-    // We will create a sequential queue so all of  the EvtIoXxx callbacks are
-    // serialized against each other (at least until the request is completed),
-    // but the cancel routine and the timer DPC are not synchronized against the
-    // queue's EvtIoXxx callbacks.
-    //
-    // attributes.SynchronizationScope = ...
-
     status = WdfDeviceCreate(&DeviceInit, &attributes, &device);
 
     if (NT_SUCCESS(status)) {
diff --git a/general/echo/kmdf/driver/DriverSync/queue.c b/general/echo/kmdf/driver/DriverSync/queue.c
index 78c199d2..7835b73b 100644
--- a/general/echo/kmdf/driver/DriverSync/queue.c
+++ b/general/echo/kmdf/driver/DriverSync/queue.c
@@ -158,6 +158,18 @@ Return Value:
     // Fill in a callback for destroy, and our QUEUE_CONTEXT size
     //
     WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&attributes, QUEUE_CONTEXT);
+
+    //
+    // By not setting the synchronization scope and using the default, there is
+    // no locking between any of the callbacks in this driver.
+    //
+    // We will create a sequential queue so all of  the EvtIoXxx callbacks are
+    // serialized against each other (at least until the request is completed),
+    // but the cancel routine and the timer DPC are not synchronized against the
+    // queue's EvtIoXxx callbacks.
+    //
+    // attributes.SynchronizationScope = ...
+
     attributes.EvtDestroyCallback = EchoEvtIoQueueContextDestroy;
 
     status = WdfIoQueueCreate(
@@ -192,7 +204,7 @@ Return Value:
         KdPrint(("WdfSpinLockCreate failed 0x%x\n",status));
         return status;
     }
-    
+
     //
     // Create the Queue timer
     //
@@ -234,16 +246,15 @@ Return Value:
     PAGED_CODE();
 
     //
-    // Create a WDFTIMER object
+    // Create a periodic timer.
+    //
+    // By not setting the synchronization scope and using the default at WdfIoQueueCreate,
+    // we are explicitly *not* serializing against the queue's lock. Instead, we will do
+    // that on our own.
     //
     WDF_TIMER_CONFIG_INIT_PERIODIC(&timerConfig, EchoEvtTimerFunc, Period);
 
     WDF_OBJECT_ATTRIBUTES_INIT(&timerAttributes);
-
-    //
-    // We are explicitly *not* serializing against the queue's lock, we will do
-    // that on our own.
-    //
     timerAttributes.ParentObject = Queue;
 
     Status = WdfTimerCreate(
@@ -456,9 +467,9 @@ EchoSetCurrentRequest(
     queueContext->CurrentStatus  = STATUS_SUCCESS;
 
     //
-    // Set the cancel routine under the lock, otherwise if we set it outside 
-    // of the lock, the timer could run and attempt to mark the request 
-    // uncancelable before we can mark it cancelable on this thread. Use 
+    // Set the cancel routine under the lock, otherwise if we set it outside
+    // of the lock, the timer could run and attempt to mark the request
+    // uncancelable before we can mark it cancelable on this thread. Use
     // WdfRequestMarkCancelableEx here to prevent to deadlock with ourselves
     // (cancel routine tries to acquire the queue object lock).
     //
@@ -746,7 +757,7 @@ Return Value:
             //
         }
     }
-    
+
     WdfSpinLockRelease(queueContext->SpinLock);
 
     //
diff --git a/general/echo/umdf2/driver/AutoSync/queue.c b/general/echo/umdf2/driver/AutoSync/queue.c
index 3162a683..98860df6 100644
--- a/general/echo/umdf2/driver/AutoSync/queue.c
+++ b/general/echo/umdf2/driver/AutoSync/queue.c
@@ -79,7 +79,7 @@ Return Value:
     // with the same lock.
     //
     queueAttributes.SynchronizationScope = WdfSynchronizationScopeQueue;
-    
+
     queueAttributes.EvtDestroyCallback = EchoEvtIoQueueContextDestroy;
 
     status = WdfIoQueueCreate(
@@ -145,21 +145,18 @@ Return Value:
     WDF_OBJECT_ATTRIBUTES  timerAttributes;
 
     //
-    // Create a WDFTIMER object
+    // Create a non-periodic timer since WDF does not allow periodic timer
+    // at passive level, which is the level UMDF callbacks are invoked at.
+    // The workaround is to always restart the timer in the timer callback.
+    //
+    // WDF_TIMER_CONFIG_INIT sets AutomaticSerialization to TRUE by default.
     //
     WDF_TIMER_CONFIG_INIT(&timerConfig, EchoEvtTimerFunc);
 
-    //
-    // WDF_OBJECT_ATTRIBUTES_INIT sets AutomaticSerialization to TRUE by default
-    //
     WDF_OBJECT_ATTRIBUTES_INIT(&timerAttributes);
     timerAttributes.ParentObject = Queue; // Synchronize with the I/O Queue
-    timerAttributes.ExecutionLevel = WdfExecutionLevelPassive; 
+    timerAttributes.ExecutionLevel = WdfExecutionLevelPassive;
 
-    //
-    // Create a non-periodic timer since WDF does not allow periodic timer 
-    // with autosynchronization at passive level
-    //
     Status = WdfTimerCreate(&timerConfig,
                              &timerAttributes,
                              Timer     // Output handle
@@ -530,7 +527,7 @@ Return Value:
     }
 
     //
-    // Restart the Timer since WDF does not allow periodic timer 
+    // Restart the Timer since WDF does not allow periodic timer
     // with autosynchronization at passive level
     //
     WdfTimerStart(Timer, WDF_REL_TIMEOUT_IN_MS(TIMER_PERIOD));