dynamic alloc inside spinlock

Pavel_S · April 22, 2012, 2:31pm

Hi all!

I would like to ask you about allocation of memory region (nonpaged) inside spinlock (same question for freeing)

Here is pseudocode:

spinlock_acquire()

mystruct *original = GetElementFromListThatNeedsSynchro();

mystruct *copy = ExAllocatePoolWithTag(NonPagedPool, sizeof(mystruct), TAG);
if (NULL != copy)
{

}

put_it_to_list(p);

spinloc

Don_Burn · April 22, 2012, 2:36pm

You can allocate non-paged pool at DISPATCH_LEVEL so unless you are
talking an interrupt spinlock, you should be ok.

Don Burn
Windows Filesystem and Driver Consulting
Website: http://www.windrvr.com
Blog: http://msmvps.com/blogs/WinDrvr

“xxxxx@gmail.com” wrote in
message news:xxxxx@ntdev:

> Hi all!
>
> I would like to ask you about allocation of memory region (nonpaged) inside spinlock (same question for freeing)
>
> Here is pseudocode:
>
> spinlock_acquire()
>
> mystruct *original = GetElementFromListThatNeedsSynchro();
>
> mystruct *copy = ExAllocatePoolWithTag(NonPagedPool, sizeof(mystruct), TAG);
> if (NULL != copy)
> {
>
> }
>
> put_it_to_list(p);
>
> spinloc

Pavel_S · April 22, 2012, 2:38pm

I’m sorry to much “spacebars” and message was posted before I finished.

Again here is a pseudocode:

spinlock_acquire()

mystruct *original = GetElementFromListThatNeedsSynchro();
if (NULL != original)
{ //element found in list)

mystruct *copy = ExAllocatePoolWithTag(NonPagedPool, sizeof(mystruct), TAG);
if (NULL != copy)
{
// here copy struct from original to copy
}
}

spinlock_release();

return copy;

Ok to idea in above code is to return new copy of elemnet from sync list to user, BUT unfortunately it is bad to alloc (or dealloc) anything in spinlock right? (BTW maybe you can create a list of WHY not to alloc memo inside locks for beginners ? ) esepcially

So I’m wondering if it wouldn’t be good to create local object, then grab spinlock check if element is in list, then copy data to local, release spinlock, alloc memo and again copy data.
Lets assuem that there is not much data (20 BYTES).

Thanks in advance!

Pavel_S · April 22, 2012, 2:41pm

One more thing. I’m posting this message to you to ask you about performance of this.

I belive that allocation or deallocation of memo still needs a lot of thing to do/check from system.

Thanks again

Daniel_Terhell · April 22, 2012, 2:44pm

If all the code plays by the rules, you technically can. But it is a bad
idea because it will increase contention. If there is any code that runs
with your lock held that you can move out from there, you should. That holds
in this example as you could do the allocation without the spinlock held.
Then there are those who have a rule of thumb that you should never make a
function call with a spinlock held.

//Daniel

wrote in message news:xxxxx@ntdev…
> Hi all!
>
> I would like to ask you about allocation of memory region (nonpaged)
> inside spinlock (same question for freeing)
>
> Here is pseudocode:
>
> spinlock_acquire()
>
> mystruct *original = GetElementFromListThatNeedsSynchro();
>
> mystruct *copy = ExAllocatePoolWithTag(NonPagedPool, sizeof(mystruct),
> TAG);
> if (NULL != copy)
> {
>
> }
>
> put_it_to_list(p);
>
> spinloc
>

Calvin_Guan-3 · April 22, 2012, 2:49pm

Don has answered your question with regard to allocating NP with spinlock
held. It’s ok.

In your case, if you are really concerned or scared of it, you can
certainly alloc the memory before taking the lock. In general, if I can
call out without a lock held, I will do it.

ie.

ptr = ExAllocXxx();
if (NULL == ptr) goto_no_copy;
spin_lock();
…
spin_unlock

On Sun, Apr 22, 2012 at 11:37 AM, wrote:

> I’m sorry to much “spacebars” and message was posted before I finished.
>
> Again here is a pseudocode:
>
>
>
> spinlock_acquire()
>
> mystruct *original = GetElementFromListThatNeedsSynchro();
> if (NULL != original)
> { //element found in list)
>
> mystruct *copy = ExAllocatePoolWithTag(NonPagedPool, sizeof(mystruct),
> TAG);
> if (NULL != copy)
> {
> // here copy struct from original to copy
> }
> }
>
> spinlock_release();
>
> return copy;
>
> Ok to idea in above code is to return new copy of elemnet from sync list
> to user, BUT unfortunately it is bad to alloc (or dealloc) anything in
> spinlock right? (BTW maybe you can create a list of WHY not to alloc memo
> inside locks for beginners ? ) esepcially
>
> So I’m wondering if it wouldn’t be good to create local object, then grab
> spinlock check if element is in list, then copy data to local, release
> spinlock, alloc memo and again copy data.
> Lets assuem that there is not much data (20 BYTES).
>
>
> Thanks in advance!
>
> —
> NTDEV is sponsored by OSR
>
> For our schedule of WDF, WDM, debugging and other seminars visit:
> http://www.osr.com/seminars
>
> To unsubscribe, visit the List Server section of OSR Online at
> http://www.osronline.com/page.cfm?name=ListServer
>

Doron_Holan · April 22, 2012, 2:58pm

Don’t worry about perf here. You are most likely overoptimizing unless this is a super hot path (which you should measure, not guess at) . Simple beats complicated every day of the week if you can go that way.

d

-----Original Message-----
From: xxxxx@lists.osr.com [mailto:xxxxx@lists.osr.com] On Behalf Of xxxxx@gmail.com
Sent: Sunday, April 22, 2012 11:40 AM
To: Windows System Software Devs Interest List
Subject: RE:[ntdev] dynamic alloc inside spinlock

One more thing. I’m posting this message to you to ask you about performance of this.

I belive that allocation or deallocation of memo still needs a lot of thing to do/check from system.

Thanks again

NTDEV is sponsored by OSR

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

To unsubscribe, visit the List Server section of OSR Online at http://www.osronline.com/page.cfm?name=ListServer

Daniel_Terhell · April 22, 2012, 3:15pm

I agree with ‘simple beats complicated’. But in this example, all he has to
do is move his allocation a few lines above. You could hardly call that a
complication or overoptimization. Even if it is not a hot path, if it
applies to good practices and common sense and you can, then why you
shouldn’t ?

//Daniel

“Calvin Guan (news)” wrote in message
news:xxxxx@ntdev…
> Don has answered your question with regard to allocating NP with spinlock
> held. It’s ok.
>
> In your case, if you are really concerned or scared of it, you can
> certainly alloc the memory before taking the lock. In general, if I can
> call out without a lock held, I will do it.
>
> ie.
>
> ptr = ExAllocXxx();
> if (NULL == ptr) goto_no_copy;
> spin_lock();
> …
> spin_unlock
>
>
>
> On Sun, Apr 22, 2012 at 11:37 AM, wrote:
>
>> I’m sorry to much “spacebars” and message was posted before I finished.
>>
>> Again here is a pseudocode:
>>
>>
>>
>> spinlock_acquire()
>>
>> mystruct *original = GetElementFromListThatNeedsSynchro();
>> if (NULL != original)
>> { //element found in list)
>>
>> mystruct *copy = ExAllocatePoolWithTag(NonPagedPool, sizeof(mystruct),
>> TAG);
>> if (NULL != copy)
>> {
>> // here copy struct from original to copy
>> }
>> }
>>
>> spinlock_release();
>>
>> return copy;
>>
>> Ok to idea in above code is to return new copy of elemnet from sync list
>> to user, BUT unfortunately it is bad to alloc (or dealloc) anything in
>> spinlock right? (BTW maybe you can create a list of WHY not to alloc memo
>> inside locks for beginners ? ) esepcially
>>
>> So I’m wondering if it wouldn’t be good to create local object, then grab
>> spinlock check if element is in list, then copy data to local, release
>> spinlock, alloc memo and again copy data.
>> Lets assuem that there is not much data (20 BYTES).
>>
>>
>> Thanks in advance!
>>
>> —
>> NTDEV is sponsored by OSR
>>
>> For our schedule of WDF, WDM, debugging and other seminars visit:
>> http://www.osr.com/seminars
>>
>> To unsubscribe, visit the List Server section of OSR Online at
>> http://www.osronline.com/page.cfm?name=ListServer
>>
>

Pavel_S · April 22, 2012, 4:01pm

Don, Daniel, Calvin, Doron

Thank you for your answers so summarizing:
In your opinien, code like this (sorry, I’m not compiling and writing fast so it is just to catch idea):

mystruct *copy = ExAllocatePoolWithTag(NonPagedPool, sizeof(mystruct), TAG);
if (NULL != copy)
{
spinlock_acquire()
mystruct *original = GetElementFromListThatNeedsSynchro();
if (NULL != original)
{ //element found in list
// here copy struct from original to copy
}
else
{
status = error;
}
spinlock_release();
}

if (status == errror)
{
ExFreePoolWithTag(copy);
}

in case when list finds (with success) will be not much, will be better than something like this:

mystruct local;
spinlock_acquire()
mystruct *original = GetElementFromListThatNeedsSynchro();
if (NULL != original)
{ //element found in list)
//make copy
status = cool
}
spinlock_release();

if (status == cool)
{
//allocate and copy again
}

Thank you in advance once again. I really appreciate your help here.

anton_bassov · April 23, 2012, 1:40am

The most recent approach with allocating and copying AFTER having released a lock is, IMHO, the most reasonable approach you have considered in so far. In any case, you seem to be over-optimizing…

Anton Bassov

James_Harper · April 23, 2012, 8:44am

>

The most recent approach with allocating and copying AFTER having released
a lock is, IMHO, the most reasonable approach you have considered in so far.
In any case, you seem to be over-optimizing…

My interpretation of the original code was that the reason the copy was done inside the spinlock and indeed the reason the copy was made at all was because the data itself was protected by the spinlock, not just the list structure…

James

anton_bassov · April 23, 2012, 9:24am

> My interpretation of the original code was that the reason the copy was done inside the

spinlock and indeed the reason the copy was made at all was because the data itself was
protected by the spinlock, not just the list structure…

This seems to be the case…

Therefore, the OP can allocate a temporary storage on the stack (he says the whole structure is just 20 bytes) and copy a data there the way he does it in the very last code sample. If he is normally able to find the data to copy the sample he provided immediately before the last one seems to be more reasonable - an extra copy that the final sample makes does not seem to make sense under these circumstances, right.

Anyway, this is “extra copy vs extra allocation/release” decision. If you are not sure about the data access patterns the final approach seems to be more reasonable - after all, unnecessarily copying 20 bytes of data obviously involves less overhead than unnecessary allocation and deallocation…

Another point to consider is whether it makes sense to worry about all this stuff, in the first place…

Anton Bassov