Hey, guys.
Can you explain for me, for what driver (this is not device driver, just driver for communication with Windows kernel) uses such type of synchronization. It calls KeAcquireSpinLock, next KeSetTimer and KeReleaseSpinLock. Would be much appricated for any suggestions. For example, before calling IoCompleteRequest, it calls KeAcquireSpinLock, checks some internal flags and depends from it values can call KeSetTimer and calls KeReleaseSpinLock. Sorry, I can’t provide more information, but maybe someone already encountered with this. Maybe this is just method of guarding timer object in SMP system.
Thank you very much.
On Oct 1, 2016, at 7:41 AM, xxxxx@gmail.com wrote:
Can you explain for me, for what driver (this is not device driver, just driver for communication with Windows kernel) uses such type of synchronization. It calls KeAcquireSpinLock, next KeSetTimer and KeReleaseSpinLock. Would be much appricated for any suggestions. For example, before calling IoCompleteRequest, it calls KeAcquireSpinLock, checks some internal flags and depends from it values can call KeSetTimer and calls KeReleaseSpinLock. Sorry, I can’t provide more information, but maybe someone already encountered with this. Maybe this is just method of guarding timer object in SMP system.
Well, you ought to be able to answer that question by looking at what else that spin lock is protecting. It’s possible this is merely an attempt to protect against the many race conditions in IRP cancellation.
I have no idea why a timer is involved. What does the timer DPC actually do?
—
Tim Roberts, xxxxx@probo.com
Providenza & Boekelheide, Inc.
It doesn’t register CancelIo function and doesn’t perform cancel I/O operations in any form. The driver registers only IRP_MJ_DEVICE_CONTROL handler (well, and IRP_MJ_CREATE). It doesn’t use DPC object in KeSetTimer.
I noticed that the code uses timer object for waiting operation (KeWaitForMultiplyObjects) only once and it was in DriverUnload function. Actually, this is very simple driver from technical point of view, because it just serves for IOCTLs dispatching. Moreover, it doesn’t perform asynchronous operations or another “hard” stuff. But this code with timer & spinlock is really important for me for understanding, because driver was written by skilled guys. 
Actually, the whole thing seems (at least to me) to smell of schizophrenia, at least as far as logic is concerned - there is something guarded by a spinlock in my code; I cannot tell you anything more and cannot show you the code either but still I want you to tell me what this spinlock actually guards.
I know someone on this thread is fond of saying that we were all inexperienced once, but this particular way of thinking seems to expose something more “exciting” than mere inexperience…
Anton Bassov
Let’s skip all words that don’t relate to subject. I just want to hear guys, who maybe encountered with such stuff or can help me bring light on this. And I started topic on this forum, because I know that this place for professionals in Windows drivers development.
The problem is that mentioned spinlock guards nothing (except, maybe, one or two dwords, I should check code again) related to usual things in such situation, but it uses spinlock & timer in same time.
I have an impression that you are looking on a code like
KeAcquireSpinLock()
{
SetTimer = CheckForTimerConditions();
if( SetTimer )
KeSetTimer();
}
KeRelaseSpinLock()
and you are wondering - can it be replaced with
KeAcquireSpinLock()
{
SetTimer = CheckForTimerConditions();
}
KeRelaseSpinLock()
if( SetTimer )
KeSetTimer();
I believe this can be done as KeSetTimer is safe for use in preemptive environment.
Thank you very much! I’ll check code after tomorrow and reply you.
I. e. you mean in first piece of code that they think that conditions of timer setting could be changed in other thread and they try to do this as atomically operation?
> The problem is that mentioned spinlock guards nothing
This changes everything - it may well happen that they use it just in order to ensure KeSetTimer()
is invoked at elevated IRQL ,i.e.use KeAcquireSpinLock() instead of KeRaiseIRQL(). In order to understand why they may want IRQL to be elevated let’s look at the sample code Slava has provided
KeAcquireSpinLock();
SetTimer = CheckForTimerConditions();
KeRelaseSpinLock();
if( SetTimer ) KeSetTimer();
Consider what happens if the above code originally runs at IRQL<dpc_level and context switch occurs after kerelasespinlock but before kesettimer invocation. therefore if duetime is specified in relative terms the timer expiry absolute may depend on external conditions that are beyond your control case above code gets executed because you don know when calling thread its chance to get hold of cpu again. certainly this just a gpos cannot provide any time-related guarantees still normally>use KeSetTimer() when meeting some soft-real-time requirements is desired. Therefore, the above excerpts seems to defeat the very purpose of using KeSetTimer().
Now let’s look at another sample
KeAcquireSpinLock() ;
SetTimer = CheckForTimerConditions();
if( SetTimer ) KeSetTimer();
KeRelaseSpinLock() ;
As you can see, the above code already does not depend on above mentioned external factors, regardless of the original IRQL, because context switches cannot occur while we are at elevated IRQL, and KeAcquireSpinLock() ensures we are going to be at DPC_LEVEL at least until we call
KeRelaseSpinLock()
Anton Bassov</dpc_level>
On Oct 1, 2016, at 12:37 PM, xxxxx@gmail.com wrote:
Let’s skip all words that don’t relate to subject. I just want to hear guys, who maybe encountered with such stuff or can help me bring light on this. And I started topic on this forum, because I know that this place for professionals in Windows drivers development.
The problem is that mentioned spinlock guards nothing (except, maybe, one or two dwords, I should check code again) related to usual things in such situation, but it uses spinlock & timer in same time.
It’s possible this code is left over from some earlier experiment. If the spin lock isn’t protecting anything, then get rid of it.
—
Tim Roberts, xxxxx@probo.com
Providenza & Boekelheide, Inc.
Frankly speaking, I don’t understand why they chose spinlock as synchronization primitive… Use KeAcquireSpinLock in IRP_MJ_DEVICE_CONTROL handler forces developers to locate its handler in NONPAGED section, that’s untypical for such drivers which doesn’t do something special eventually. From my point of view critical section is more a worthy candidate.
Disagree. Given that you can’t guarantee the IRQL at which your IOCTL dispatch routine will be called, and the fact that spin locks are very efficient, i would say a spin lock is a good and common choice. In terms of whether dispatch routines are ordinarily non-paged, I’d also disagree. Unless there’s a specific reason to do so, I *never* make this type of code non-paged. Just no point I. Doing so, and way too many ways to screw it up.
Peter
OSR
@OSRDrivers
I believe that documentation for the driver should declare IRQL level on which the handler can be called, like it implemented in WDK. But in my case, this is 100% guarantee that handler will called on PASSIVE level. If someone wants to call it on >PASSIVE level, its own problem, because this is undocumented measure. All undocumented calls in kernel mode lead to BSOD.
I think you better than me know that even WDK declares rather narrow range of operations which should involve spinlocks, due to their overhead for system performance. And this case is definitely not belong to that range.
Spinlocks overhead is mostly overrated. Spinlocks overhead manifests itself only on highly contention resources when multiple CPUs contest for it to get read access, in case of write access you don’t have much choice. The spinlock overhead consists of
- committing(retiring) prior memory instructions on CPU as spinlock involves a memory barrier, this might cause some cache coherency traffic, but it happens anyway with or w/o spinlock as eventually these instructions are retired sooner or later. This also involves some performance hit as might hinder out-of-order and speculative execution by stalling the pipeline.
- communicating with all CPU cache controllers to lock the contested cache line, if there is no contention there is nothing to update/invalidate in remote CPU caches as the line is already in exclusive possession
- reading a value from the locked cache line and writing a new value back
- unlocking the locked CPU cache line
To avoid the above penalties in case of data that is mostly read by multiple threads some OSes employ RCU( Read-Copy-Update). But even RCU requires a spin lock or similar synchronization primitive for write access in case of multiple writers. Windows has queued spinlocks that generate reduced CPU cache coherency traffic and provide fairness in exchange of additional memory requirements, but there is a theorem in CS that states that you can’t provide fairness for nothing.
@Slava Imameev
Could you pls give more detailed explanation about your first post? I mean situations in which such code should be used.
It should be used whenever CheckForTimerConditions() is not “thread safe”. There might be more than one file object opened for a driver’s control device object or there might be an asynchronous file object, in the latter case the IO Manager doesn’t serialize access for a file object. As a result of these two scenarios there might be multiple threads executing CheckForTimerConditions() simultaneously. You can call KeSetTimer() inside or outside spinlock protected section, the latter is “thread safe” because of internal KeSetTimer() synchronization and SetTimer being a thread local variable. You can replace the spinlock with synchronization method of your choice.
We will have to agree to disagree, Mr Baranov. On, well, just about everything you said in your post above.
Peter
OSR
@OSRDrivers
> I believe that documentation for the driver should declare IRQL level on which the handler
can be called, like it implemented in WDK.
This may apply only to standalone drivers that don’t attach themselves to any device stacks. In this case they may, indeed, make some assumptions about the callers. Drivers of this type are normally meant to work only alongside some particular application that may want to gain an access to the kernel address space, and, hence, are designed to receive IRPs only in context of app’s calls to Read/Write/IOCTL userland routines.Furthermore, they have to be legacy ones controlled by SCM.
However, as long as you attach your drver to some device stack you already have to play by this stack’s rules.
Consider the situation when a hardware driver at the bottom of the stack completes an IRP in context of DPC. In this case a chain of registered completion routines will be invoked at the way up that stack at DISPATCH_LEVEL. If some driver’s completion routine decides to send a new IRP down the stack, the corresponding dispatch routines of all drivers in the stack that are below the caller are going to get invoked, and this is going to happen at DISPATCH_LEVEL as well. In other situations these same routines may be invoked at PASSIVE_LEVEL. This is why Peter said that you have no control over caller’s IRQL whatsoever.
I think you better than me know that even WDK declares rather narrow range of operations
which should involve spinlocks, due to their overhead for system performance.
But you DO realize that the use of any dispatcher-level construct (event,fast mutex,semaphore,etc)
inevitably involves spinlock acquisition and release behind the scenes,right…
Anton Bassov
@Peter
“In terms of whether dispatch
routines are ordinarily non-paged, I’d also disagree. Unless there’s a specific
reason to do so, I *never* make this type of code non-paged. Just no point I.
Doing so, and way too many ways to screw it up.”
Did you mean to say you *never* make this type of code *paged*?
@Anton Bassov
Pls reread this topic more carefully, I’ve already pointed that this is simple standalone driver that used by the client only for ioctls dispatching.
I’ve checked code again and looks like spinlock guards some DeviceExtension fields.
The driver uses next piece of code in the end of IRP_MJ_DEVICE_CONTROL handler.
KeAcquireSpinLock();
Flag1 = DeviceExtension->Flag1;
Flag2 = DeviceExtension->Flag2;
if( Flag1 & Flag2 ) {
KeSetTimer();
}
KeRelaseSpinLock();
It also uses next piece of code in the BEGIN of IRP_MJ_DEVICE_CONTROL handler.
KeAcquireSpinLock();
Flag1 = DeviceExtension->Flag1;
Flag2 = DeviceExtension->Flag2;
if( Flag1 & Flag2 ) {
KeCancelTimer();
}
KeRelaseSpinLock();