multiple spinlocks and KeLowerIrql()... really?!

I have a pair of spinlocks and am hoping for some clarification of whether this is correct/ safe. Is this a normalish idiom for dealing with multiple spinlocks?.. it seems a bit offbeat to me… hm

KIRQL irqlOnEntry = KeGetCurrentIrql();

WdfSpinLockAcquire(Lock1);
WdfSpinLockAcquire(Lock2);
DoSomethingRequiringLock1And2Held();

WdfSpinLockRelease(1);
// now at irqlOnEntry

// is it ok to lower irql by side effect while we are holding Lock2?

DoSomethingRequiringLock2Held();

WdfSpinLockRelease(Lock2);
// now at dispatch because that was the irql when we acquired
// but we need to restore the orignal, original irql

KeLowerIrql(irqlOnEntry); // shudder!

return;

As WdfSpinLockAcquire is really just a thin wrapper around
KeAcquireSpinlock (and same for lockrelease) I would use the legacy
interfaces here to do the right thing with respect to irql levels. This
assumes that for other reasons you need the lock order to be 1…2 rather
than 2…1.

Mark Roddy

On Tue, Sep 12, 2017 at 9:12 AM, xxxxx@gmail.com <
xxxxx@lists.osr.com> wrote:

I have a pair of spinlocks and am hoping for some clarification of whether
this is correct/ safe. Is this a normalish idiom for dealing with multiple
spinlocks?.. it seems a bit offbeat to me… hm

KIRQL irqlOnEntry = KeGetCurrentIrql();

WdfSpinLockAcquire(Lock1);
WdfSpinLockAcquire(Lock2);
DoSomethingRequiringLock1And2Held();

WdfSpinLockRelease(1);
// now at irqlOnEntry

// is it ok to lower irql by side effect while we are holding Lock2?

DoSomethingRequiringLock2Held();

WdfSpinLockRelease(Lock2);
// now at dispatch because that was the irql when we acquired
// but we need to restore the orignal, original irql

KeLowerIrql(irqlOnEntry); // shudder!

return;


NTDEV is sponsored by OSR

Visit the list online at: http:> showlists.cfm?list=ntdev>
>
> MONTHLY seminars on crash dump analysis, WDF, Windows internals and
> software drivers!
> Details at http:
>
> To unsubscribe, visit the List Server section of OSR Online at <
> http://www.osronline.com/page.cfm?name=ListServer&gt;
></http:></http:>

Thanks Mark - I did consider this; but the documentation for KeReleaseSpinLock says I must supply the IRQL returned from the associated call to AcquireSpinLock; humph – A colleague came up with the novel suggestion of a third spinlock to “scope” the other two

ie the IRQL would be correct in my snippet and releasing the third one would restore IRQL correctly. I need a big black cup of coffee to decide whether this is brilliant or whether it is a large sledge hammer to crack a small nut :slight_smile:

jolyon

That will be bad, as when you release Lock1, you return to the IRQL things were at before acquiring Lock1, which may be PASSIVE_LEVEL. If you then just dropped to PASSIVE_LEVEL, you are executing with Lock2 held while at PASSIVE_LEVE. Let’s then say a DPC fires, interrupting you code at PASSIVE_LEVEL, that holds Lock2, and that DPC tried to acquire Lock2. Your system is now deadlocked, as the DPC can’t acquire Lock2, and the PASSIVE_LEVEL thread will stay interrupted by the DPC, so is never able to release Lock2.

A workaround could be to manually raise the IRQL to DISPATCH_LEVEL before acquiring Lock1.

Adding a third spinlock around everything will indirectly guarantee you acquire Lock1 and Lock2 at DISPATCH_LEVEL, assuring both Lock1 and Lock2 release to the same IRQL, so you can keep using the KMDF lock APIs.

I usually try really hard to keep locks very consistent, as having lock special cases just makes future maintenance more error prone.

Jan

-----Original Message-----
From: xxxxx@lists.osr.com [mailto:xxxxx@lists.osr.com] On Behalf Of xxxxx@gmail.com xxxxx@lists.osr.com
Sent: Tuesday, September 12, 2017 6:12 AM
To: Windows System Software Devs Interest List
Subject: [ntdev] multiple spinlocks and KeLowerIrql()… really?!

I have a pair of spinlocks and am hoping for some clarification of whether this is correct/ safe. Is this a normalish idiom for dealing with multiple spinlocks?.. it seems a bit offbeat to me… hm

KIRQL irqlOnEntry = KeGetCurrentIrql();

WdfSpinLockAcquire(Lock1);
WdfSpinLockAcquire(Lock2);
DoSomethingRequiringLock1And2Held();

WdfSpinLockRelease(1);
// now at irqlOnEntry

// is it ok to lower irql by side effect while we are holding Lock2?

DoSomethingRequiringLock2Held();

WdfSpinLockRelease(Lock2);
// now at dispatch because that was the irql when we acquired
// but we need to restore the orignal, original irql

KeLowerIrql(irqlOnEntry); // shudder!

return;


NTDEV is sponsored by OSR

Visit the list online at: http:

MONTHLY seminars on crash dump analysis, WDF, Windows internals and software drivers!
Details at http:

To unsubscribe, visit the List Server section of OSR Online at http:</http:></http:></http:>

Yes, you should release locks in the reverse order. As Jan said, if lock 1 is acquired at passive level and if you release lock 1 before lock 2 then you will hold lock 2 at passive level which is a fatal error.

Look at KeLowerIrql’s documentation, you have: “It is a fatal error to call KeLowerIrql using an input NewIrql that was not returned by the immediately preceding call to KeRaiseIrql”.

I don’t know if this code passes Static Driver Verifier analysis but you should test it.

What you should do is raise IRQL to DISPATCH_LEVEL on entry, and lower it back on exit. Then IRQL will be consistent. You’ll also be able to use a bit faster variants of spinlock calls which don’t do IRQL change.

thanks for your thoughts folks - I am playing with the Spinlock Three idea which is currently looking good… I figure that anything is better than directly manipulating IRQL… even juggling another spinlock!

The reason for the release order is dictated by another part of the code - I am trying to avoid deadlock and excessive changes to existing code

I am in error prone maintenance mode :slight_smile:

today anyway - tomorrow, of course, i will be in everything works like a charm mode (ho ho)

thanks again,

jolyon

> -----Original Message-----

Sent: Tuesday, September 12, 2017 9:35 AM
Subject: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!

thanks for your thoughts folks - I am playing with the Spinlock Three idea
which is currently looking good… I figure that anything is better than
directly manipulating IRQL… even juggling another spinlock!

The reason for the release order is dictated by another part of the code -
I am trying to avoid deadlock and excessive changes to existing code

If you don’t wrap every reference to locks 1 and/or 2 inside 3, you still run the risk of deadlock.

Either always release in reverse order, or just use one lock. Anything else is a partially loaded revolver.

Phil
Not speaking for LogRhythm
Philip D Barila
Senior Software Engineer
720.881.5364 (W) ?
LogRhythm.com
?

?

?

?

If the documentation says that, it’s mistaken and needs fixed.

It is very silly to use an additional spin lock in this case.

Your choices to me are clear:

a) Do as Mr. Roddy suggests. This works, it’s always worked, and always will work. If the documentation suggests otherwise, file a bug.

b) Always release the spin locks in the inverse order that you acquire them.

Option A, above, has the advantage of being more technically concise. But it requires you write a lot of comments to explain what you’re doing. In my experience, when you quick read (correct) code that implements option A, it LOOKS like a bug… so you’ll need to excessively comment your actions.

Option B, above, has the advantage of being instantly understandable during maintenance. But it has the disadvantage of being somewhat less efficient.

All things considered, unless the efficiencies gained by Option A are significant enough to outweigh the disadvantages, just bite the bullet and go with Option B.

Peter
OSR
@OSRDrivers

I also think that a third spin is not the right decision. Holding two spin locks is not recommended, so holding three is not something you should try.

You can’t do what you want at DISPATCH_LEVEL. Bug check 0x133 with first parameter set to 1 is trigerred when “the system cumulatively spent an extended period of time at IRQL DISPATCH_LEVEL”.

You should move toward the single lock direction. Typically lock L1 protects access to resource R1 while lock L2 protects access to resource R2. If you have to access R2 while L1 is held than you should consider that resources R1 and R2 make the same resource R and that a single lock L should be acquired whenever L1 or L2 (that is L) is accessed.

>…but the documentation for KeReleaseSpinLock says I must supply the IRQL

returned from the associated call to AcquireSpinLock;

KeAcquireSpinLock() is,basically, just a wrapper for the following sequence:

old_irql=KeRaiseIrql(DISPATCH_LEVEL);
KeAcquireSpinLocAt DpcLevel();
* p_old_irql=old_irql;

Therefore,KeReleaseSpinLock() does the following:

KeReleaseSpinLockFromDpcLevel();
KeLowerIqrl( old_irql);

I hope the above lines are sufficient for realising that, although you can release spinlocks in any order that you wish, irql parameter should be provided in the reverse order of lock acquisition, unless you are absolutely sure the first lock always gets acquired at elevated IRQL (for example, you acquire all your locks in context of DPC routine).

In your particular case you should do the following ( I assume you are not doing it in context of DPC routine):

KeAcquireSpinLock(& lock1, &old_irql1);

KeAcquireSpinLock(& lock2, &old_irql2);

KeReleaseSpinLock(lock1, old_irql2);
KeReleaseSpinLock(lock2, old_irql1);

Alternatively, you can simply call KeRaiseIrql(DISPATCH_LEVEL) before you proceed to spinlock acquisition and KeLowerIrql()after both locks have been released. In such case you can simply ignore old_irql parameter, because it becomes meaningless in this scenario (or, even better, use XXXXDpcLevel() routines). I think this approach is better simply because it saves you extra confusion.

What you should NEVER do is the following( again, unless you are 100% sure you always do it at elevated IRQL)

KeAcquireSpinLock(& lock1, &old_irql1);

KeAcquireSpinLock(& lock2, &old_irql2);

KeReleaseSpinLock(lock1, old_irql1;
KeReleaseSpinLock(lock2, old_irql2);

If you do it this way and your original IRQL is not elevated you will end up with a spinlock held at IRQL<dispatch_level which is a fatal error because it may result in deadlock>
Anton Bassov</dispatch_level>

It really depends…

Although the above is true in vast majority of the cases there are some scenarios when you may want to achieve finer-grained locking. There are some (admittedly rare, but still existing) situations
when having a single lock will result in a “hot lock” that is heavily contended for, while nested locking will lead to the situation when all the locks in a chain may be acquired without any contention in vast majority of cases(albeit at the cost of extra code complexity).

In other words, this is just a usual dilemma of whether potential improvements in terms of efficiency justify the additional code complexity

Anton Bassov

While there are a few legitimate cases for releasing locks in an order that is different than the acquisition order, almost certainly this is the wrong approach

The use of a third lock solution, is equivalent to replacing lock 1 & lock 2 with lock 3 and might help you with a short term problem hos to fix broken code, but is not a solution to the multiple lock problem

If you can tell us more, we may be able to help you better, but otherwise the general rule is don?t do this.

Sent from Mailhttps: for Windows 10

From: Phil Barilamailto:xxxxx
Sent: September 12, 2017 11:46 AM
To: Windows System Software Devs Interest Listmailto:xxxxx
Subject: RE: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!

> -----Original Message-----
> Sent: Tuesday, September 12, 2017 9:35 AM
> Subject: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!
>
> thanks for your thoughts folks - I am playing with the Spinlock Three idea
> which is currently looking good… I figure that anything is better than
> directly manipulating IRQL… even juggling another spinlock!
>
> The reason for the release order is dictated by another part of the code -
> I am trying to avoid deadlock and excessive changes to existing code

If you don’t wrap every reference to locks 1 and/or 2 inside 3, you still run the risk of deadlock.

Either always release in reverse order, or just use one lock. Anything else is a partially loaded revolver.

Phil
Not speaking for LogRhythm
Philip D Barila
Senior Software Engineer
720.881.5364 (W)
LogRhythm.com


NTDEV is sponsored by OSR

Visit the list online at: http:

MONTHLY seminars on crash dump analysis, WDF, Windows internals and software drivers!
Details at http:

To unsubscribe, visit the List Server section of OSR Online at http:</http:></http:></http:></mailto:xxxxx></mailto:xxxxx></https:>

There really is nothing wrong with unordered *lock* release as long as in
NT you maintain the correct irql levels. Ordering is required on
acquisition, not release. But if you can demonstrate an example of
wrongness about unordered release I’d be happy to revise my view on this.

Mark Roddy

On Tue, Sep 12, 2017 at 4:49 PM, Marion Bond <
xxxxx@lists.osr.com> wrote:

> While there are a few legitimate cases for releasing locks in an order
> that is different than the acquisition order, almost certainly this is the
> wrong approach
>
>
>
> The use of a third lock solution, is equivalent to replacing lock 1 & lock
> 2 with lock 3 and might help you with a short term problem hos to fix
> broken code, but is not a solution to the multiple lock problem
>
>
>
> If you can tell us more, we may be able to help you better, but otherwise
> the general rule is don’t do this.
>
>
>
> Sent from Mail https: for
> Windows 10
>
>
>
> *From: *Phil Barila
> *Sent: *September 12, 2017 11:46 AM
> *To: *Windows System Software Devs Interest List
> *Subject: *RE: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!
>
>
>
> > -----Original Message-----
> > Sent: Tuesday, September 12, 2017 9:35 AM
> > Subject: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!
> >
> > thanks for your thoughts folks - I am playing with the Spinlock Three
> idea
> > which is currently looking good… I figure that anything is better than
> > directly manipulating IRQL… even juggling another spinlock!
> >
> > The reason for the release order is dictated by another part of the code
> -
> > I am trying to avoid deadlock and excessive changes to existing code
>
> If you don’t wrap every reference to locks 1 and/or 2 inside 3, you still
> run the risk of deadlock.
>
> Either always release in reverse order, or just use one lock. Anything
> else is a partially loaded revolver.
>
> Phil
> Not speaking for LogRhythm
> Philip D Barila
> Senior Software Engineer
> 720.881.5364 <(720)%20881-5364> (W)
> LogRhythm.com
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> —
> NTDEV is sponsored by OSR
>
> Visit the list online at: http:> showlists.cfm?list=ntdev>
>
> MONTHLY seminars on crash dump analysis, WDF, Windows internals and
> software drivers!
> Details at http:
>
> To unsubscribe, visit the List Server section of OSR Online at <
> http://www.osronline.com/page.cfm?name=ListServer&gt;
>
>
>
> —
> NTDEV is sponsored by OSR
>
> Visit the list online at: http:> showlists.cfm?list=ntdev>
>
> MONTHLY seminars on crash dump analysis, WDF, Windows internals and
> software drivers!
> Details at http:
>
> To unsubscribe, visit the List Server section of OSR Online at <
> http://www.osronline.com/page.cfm?name=ListServer&gt;
></http:></http:></http:></http:></https:>

Leave ?wrongness? aside for a moment and think about ?usefulness?. Perhaps you can think better than me, but when is it useful to release locks out of order?

I have done this in a few cases where the locking scheme was very complex and there was an interaction between the data structure that must be protected from corruption and the data element that is being accessed, but the general rule is still don?t do it ? if for no other reason that it will leave others very much confused and likely to break your code. Without a clear benefit of some kind, this is a good reason not to

Sent from Mailhttps: for Windows 10

From: Mark Roddymailto:xxxxx
Sent: September 12, 2017 6:04 PM
To: Windows System Software Devs Interest Listmailto:xxxxx
Subject: Re: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!

There really is nothing wrong with unordered lock release as long as in NT you maintain the correct irql levels. Ordering is required on acquisition, not release. But if you can demonstrate an example of wrongness about unordered release I’d be happy to revise my view on this.

Mark Roddy

On Tue, Sep 12, 2017 at 4:49 PM, Marion Bond > > wrote:
While there are a few legitimate cases for releasing locks in an order that is different than the acquisition order, almost certainly this is the wrong approach

The use of a third lock solution, is equivalent to replacing lock 1 & lock 2 with lock 3 and might help you with a short term problem hos to fix broken code, but is not a solution to the multiple lock problem

If you can tell us more, we may be able to help you better, but otherwise the general rule is don?t do this.

Sent from Mailhttps: for Windows 10

From: Phil Barilamailto:xxxxx
Sent: September 12, 2017 11:46 AM
To: Windows System Software Devs Interest Listmailto:xxxxx
Subject: RE: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!

> -----Original Message-----
> Sent: Tuesday, September 12, 2017 9:35 AM
> Subject: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!
>
> thanks for your thoughts folks - I am playing with the Spinlock Three idea
> which is currently looking good… I figure that anything is better than
> directly manipulating IRQL… even juggling another spinlock!
>
> The reason for the release order is dictated by another part of the code -
> I am trying to avoid deadlock and excessive changes to existing code

If you don’t wrap every reference to locks 1 and/or 2 inside 3, you still run the risk of deadlock.

Either always release in reverse order, or just use one lock. Anything else is a partially loaded revolver.

Phil
Not speaking for LogRhythm
Philip D Barila
Senior Software Engineer
720.881.5364tel: (W)
LogRhythm.com


NTDEV is sponsored by OSR

Visit the list online at: http:

MONTHLY seminars on crash dump analysis, WDF, Windows internals and software drivers!
Details at http:

To unsubscribe, visit the List Server section of OSR Online at http:


NTDEV is sponsored by OSR

Visit the list online at: http:

MONTHLY seminars on crash dump analysis, WDF, Windows internals and software drivers!
Details at http:

To unsubscribe, visit the List Server section of OSR Online at http:

— NTDEV is sponsored by OSR Visit the list online at: MONTHLY seminars on crash dump analysis, WDF, Windows internals and software drivers! Details at To unsubscribe, visit the List Server section of OSR Online at</http:></http:></http:></http:></http:></http:></tel:></mailto:xxxxx></mailto:xxxxx></https:></mailto:xxxxx></mailto:xxxxx></https:>

The case was already discussed: resource1 is “hot” and its lock needs to be
held to safely acquire the lock for resource2. Having acquired the lock for
resource2 you want to drop the lock for resource1.

Mark Roddy

On Tue, Sep 12, 2017 at 6:55 PM, Marion Bond <
xxxxx@lists.osr.com> wrote:

> Leave ‘wrongness’ aside for a moment and think about ‘usefulness’.
> Perhaps you can think better than me, but when is it useful to release
> locks out of order?
>
>
>
> I have done this in a few cases where the locking scheme was very
> complex and there was an interaction between the data structure that must
> be protected from corruption and the data element that is being accessed,
> but the general rule is still don’t do it – if for no other reason that it
> will leave others very much confused and likely to break your code.
> Without a clear benefit of some kind, this is a good reason not to
>
>
>
>
>
>
>
> Sent from Mail https: for
> Windows 10
>
>
>
> *From: *Mark Roddy
> *Sent: *September 12, 2017 6:04 PM
> *To: *Windows System Software Devs Interest List
> *Subject: *Re: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!
>
>
>
> There really is nothing wrong with unordered lock release as long as in
> NT you maintain the correct irql levels. Ordering is required on
> acquisition, not release. But if you can demonstrate an example of
> wrongness about unordered release I’d be happy to revise my view on this.
>
>
> Mark Roddy
>
>
>
> On Tue, Sep 12, 2017 at 4:49 PM, Marion Bond <
> xxxxx@lists.osr.com> wrote:
>
> While there are a few legitimate cases for releasing locks in an order
> that is different than the acquisition order, almost certainly this is the
> wrong approach
>
>
>
> The use of a third lock solution, is equivalent to replacing lock 1 & lock
> 2 with lock 3 and might help you with a short term problem hos to fix
> broken code, but is not a solution to the multiple lock problem
>
>
>
> If you can tell us more, we may be able to help you better, but otherwise
> the general rule is don’t do this.
>
>
>
> Sent from Mail https: for
> Windows 10
>
>
>
> *From: *Phil Barila
> *Sent: *September 12, 2017 11:46 AM
> *To: *Windows System Software Devs Interest List
> *Subject: *RE: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!
>
>
>
> > -----Original Message-----
> > Sent: Tuesday, September 12, 2017 9:35 AM
> > Subject: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!
> >
> > thanks for your thoughts folks - I am playing with the Spinlock Three
> idea
> > which is currently looking good… I figure that anything is better than
> > directly manipulating IRQL… even juggling another spinlock!
> >
> > The reason for the release order is dictated by another part of the code
> -
> > I am trying to avoid deadlock and excessive changes to existing code
>
> If you don’t wrap every reference to locks 1 and/or 2 inside 3, you still
> run the risk of deadlock.
>
> Either always release in reverse order, or just use one lock. Anything
> else is a partially loaded revolver.
>
> Phil
> Not speaking for LogRhythm
> Philip D Barila
> Senior Software Engineer
> 720.881.5364 <(720)%20881-5364> (W)
> LogRhythm.com
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> —
> NTDEV is sponsored by OSR
>
> Visit the list online at: http:> showlists.cfm?list=ntdev>
>
> MONTHLY seminars on crash dump analysis, WDF, Windows internals and
> software drivers!
> Details at http:
>
> To unsubscribe, visit the List Server section of OSR Online at <
> http://www.osronline.com/page.cfm?name=ListServer&gt;
>
>
>
>
> —
> NTDEV is sponsored by OSR
>
> Visit the list online at: http:> showlists.cfm?list=ntdev>
>
> MONTHLY seminars on crash dump analysis, WDF, Windows internals and
> software drivers!
> Details at http:
>
> To unsubscribe, visit the List Server section of OSR Online at <
> http://www.osronline.com/page.cfm?name=ListServer&gt;
>
>
>
> — NTDEV is sponsored by OSR Visit the list online at: MONTHLY seminars
> on crash dump analysis, WDF, Windows internals and software drivers!
> Details at To unsubscribe, visit the List Server section of OSR Online at
>
>
>
> —
> NTDEV is sponsored by OSR
>
> Visit the list online at: http:> showlists.cfm?list=ntdev>
>
> MONTHLY seminars on crash dump analysis, WDF, Windows internals and
> software drivers!
> Details at http:
>
> To unsubscribe, visit the List Server section of OSR Online at <
> http://www.osronline.com/page.cfm?name=ListServer&gt;
></http:></http:></http:></http:></http:></http:></https:></https:>

I’d agree a third spinlock is silly, EXCEPT this might be a driver that requires pure WDF calls to pass WHQL certification, in which case you can’t just call the native IRQL control functions. I’m not sure what the current status on requiring pure WDF drivers is for certification. I thought some classes of devices were flexible, and you were free to use many non WDF APIs, but some device classes were not flexible and calling any non WDF APIs is instant certification failure.

I’m still pondering if I agree with Phil’s comment that using the 3rd outer spinlock will require always locking that third spinlock. Offhand, it’s not obvious to me why this is a problem, assuming the third spinlock is only used in this case, as an indirect way to assure DISPATCH_LEVEL when Lock1 is acquired. The third spinlock makes me uncomfortable, but at the same time I can’t offhand describe the exact case where it’s a problem. I have been writing JavaScript code for a week, which is all single threaded, so my brain may be on a concurrency vacation.

Certainly the best solution would be arrange the code so the two spinlocks are released in lifo order. Schedule pressure might be such that a temporary fix now is more appropriate than a better fix in a few days. If course once the fire is out, the priority of going back and doing a better fix may not be very high.

Jan

-----Original Message-----
From: xxxxx@lists.osr.com [mailto:xxxxx@lists.osr.com] On Behalf Of xxxxx@osr.com xxxxx@lists.osr.com
Sent: Tuesday, September 12, 2017 9:39 AM
To: Windows System Software Devs Interest List
Subject: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!



If the documentation says that, it’s mistaken and needs fixed.

It is very silly to use an additional spin lock in this case.

Your choices to me are clear:

a) Do as Mr. Roddy suggests. This works, it’s always worked, and always will work. If the documentation suggests otherwise, file a bug.

b) Always release the spin locks in the inverse order that you acquire them.

Option A, above, has the advantage of being more technically concise. But it requires you write a lot of comments to explain what you’re doing. In my experience, when you quick read (correct) code that implements option A, it LOOKS like a bug… so you’ll need to excessively comment your actions.

Option B, above, has the advantage of being instantly understandable during maintenance. But it has the disadvantage of being somewhat less efficient.

All things considered, unless the efficiencies gained by Option A are significant enough to outweigh the disadvantages, just bite the bullet and go with Option B.

Peter
OSR
@OSRDrivers


NTDEV is sponsored by OSR

Visit the list online at: http:

MONTHLY seminars on crash dump analysis, WDF, Windows internals and software drivers!
Details at http:

To unsubscribe, visit the List Server section of OSR Online at http:</http:></http:></http:>

I don’t know of any WHQL test that requires “pure WDF”. If there are then
the WDF doc team should change the docs that clearly state that using WDM
interfaces is fine. For example:
https://docs.microsoft.com/en-us/windows-hardware/drivers/wdf/accessing-wdm-interfaces-in-kmdf-drivers

Mark Roddy

On Tue, Sep 12, 2017 at 9:30 PM, Jan Bottorff <
xxxxx@lists.osr.com> wrote:

> I’d agree a third spinlock is silly, EXCEPT this might be a driver that
> requires pure WDF calls to pass WHQL certification, in which case you can’t
> just call the native IRQL control functions. I’m not sure what the current
> status on requiring pure WDF drivers is for certification. I thought some
> classes of devices were flexible, and you were free to use many non WDF
> APIs, but some device classes were not flexible and calling any non WDF
> APIs is instant certification failure.
>
> I’m still pondering if I agree with Phil’s comment that using the 3rd
> outer spinlock will require always locking that third spinlock. Offhand,
> it’s not obvious to me why this is a problem, assuming the third spinlock
> is only used in this case, as an indirect way to assure DISPATCH_LEVEL when
> Lock1 is acquired. The third spinlock makes me uncomfortable, but at the
> same time I can’t offhand describe the exact case where it’s a problem. I
> have been writing JavaScript code for a week, which is all single threaded,
> so my brain may be on a concurrency vacation.
>
> Certainly the best solution would be arrange the code so the two
> spinlocks are released in lifo order. Schedule pressure might be such that
> a temporary fix now is more appropriate than a better fix in a few days. If
> course once the fire is out, the priority of going back and doing a better
> fix may not be very high.
>
> Jan
>
> -----Original Message-----
> From: xxxxx@lists.osr.com [mailto:bounce-637793-145174@
> lists.osr.com] On Behalf Of xxxxx@osr.com xxxxx@lists.osr.com
> Sent: Tuesday, September 12, 2017 9:39 AM
> To: Windows System Software Devs Interest List
> Subject: RE:[ntdev] multiple spinlocks and KeLowerIrql()… really?!
>
>


>
> If the documentation says that, it’s mistaken and needs fixed.
>
> It is very silly to use an additional spin lock in this case.
>
> Your choices to me are clear:
>
> a) Do as Mr. Roddy suggests. This works, it’s always worked, and always
> will work. If the documentation suggests otherwise, file a bug.
>
> b) Always release the spin locks in the inverse order that you acquire
> them.
>
> Option A, above, has the advantage of being more technically concise. But
> it requires you write a lot of comments to explain what you’re doing. In
> my experience, when you quick read (correct) code that implements option A,
> it LOOKS like a bug… so you’ll need to excessively comment your actions.
>
> Option B, above, has the advantage of being instantly understandable
> during maintenance. But it has the disadvantage of being somewhat less
> efficient.
>
> All things considered, unless the efficiencies gained by Option A are
> significant enough to outweigh the disadvantages, just bite the bullet and
> go with Option B.
>
> Peter
> OSR
> @OSRDrivers
>
>
> —
> NTDEV is sponsored by OSR
>
> Visit the list online at: http:> showlists.cfm?list=ntdev>
>
> MONTHLY seminars on crash dump analysis, WDF, Windows internals and
> software drivers!
> Details at http:
>
> To unsubscribe, visit the List Server section of OSR Online at <
> http://www.osronline.com/page.cfm?name=ListServer&gt;
>
> —
> NTDEV is sponsored by OSR
>
> Visit the list online at: http:> showlists.cfm?list=ntdev>
>
> MONTHLY seminars on crash dump analysis, WDF, Windows internals and
> software drivers!
> Details at http:
>
> To unsubscribe, visit the List Server section of OSR Online at <
> http://www.osronline.com/page.cfm?name=ListServer&gt;
></http:></http:></http:></http:>

The use of WDM spin locks in KMDF is legitimate but my understanding of the “Spinlock rule (kmdf)” is that you can’t hold two spin locks:

“The Spinlock rule specifies that calls to KeAcquireSpinLock or KeAcquireSpinLockRaiseToDpc and KeReleaseSpinlock are used in strict alternation.”

https://msdn.microsoft.com/en-us/library/windows/hardware/ff819092(v=vs.85).aspx

The same apply for KMDF spin locks:

“The WdfSpinlock rule specifies that calls to the WdfSpinLockAcquire method are used in strict alternation with WdfSpinlockRelease. At the end of any KMDF callback routine, the driver should not hold the framework spinlock object that was obtained by a previous call to WdfSpinLockAcquire.”

https://msdn.microsoft.com/en-us/library/windows/hardware/ff556105(v=vs.85).aspx

To me “strict alternation” means that you must call KeReleaseSpinlock or WdfSpinlockRelease before calling KeAcquireSpinLock(RaiseToDpc) or WdfSpinLockAcquire again.

I have no idea what that means, but it would be crazy wrong to require “no
more than one lock held at a time”. Consider an object that contains a
collection of objects. Forcing the locking to the container object’s lock
for each item in the collection is idiotic.

The restriction on not holding a lock at the completion of a callback is
fine.

Mark Roddy

On Wed, Sep 13, 2017 at 11:16 AM, xxxxx@gmail.com
wrote:

> The use of WDM spin locks in KMDF is legitimate but my understanding of
> the “Spinlock rule (kmdf)” is that you can’t hold two spin locks:
>
> “The Spinlock rule specifies that calls to KeAcquireSpinLock or
> KeAcquireSpinLockRaiseToDpc and KeReleaseSpinlock are used in strict
> alternation.”
>
> https://msdn.microsoft.com/en-us/library/windows/hardware/
> ff819092(v=vs.85).aspx
>
> The same apply for KMDF spin locks:
>
> “The WdfSpinlock rule specifies that calls to the WdfSpinLockAcquire
> method are used in strict alternation with WdfSpinlockRelease. At the end
> of any KMDF callback routine, the driver should not hold the framework
> spinlock object that was obtained by a previous call to WdfSpinLockAcquire.”
>
> https://msdn.microsoft.com/en-us/library/windows/hardware/
> ff556105(v=vs.85).aspx
>
> To me “strict alternation” means that you must call KeReleaseSpinlock or
> WdfSpinlockRelease before calling KeAcquireSpinLock(RaiseToDpc) or
> WdfSpinLockAcquire again.
>
>
> —
> NTDEV is sponsored by OSR
>
> Visit the list online at: http:> showlists.cfm?list=ntdev>
>
> MONTHLY seminars on crash dump analysis, WDF, Windows internals and
> software drivers!
> Details at http:
>
> To unsubscribe, visit the List Server section of OSR Online at <
> http://www.osronline.com/page.cfm?name=ListServer&gt;
></http:></http:>