Help me for a typical problem regarding KeAquireSpinLock which is not Locking other Thread in Same C

Discussion in 'Windows Vista Drivers' started by Rohit, Apr 25, 2006.

  1. Rohit

    Rohit Guest

    Hi All,
    I have a typical problem regarding KeAquireSpinLock which is not
    Blocking other Threads in Same CPU.
    The Scenario is I've created 2 threads and in the first thread i call
    KeAquireSpinLock to the same global KSPIN_LOCK variable, while its
    locked by the first thread which is running the critical section, i
    call the same lock in the second thread, and that also gets the lock
    and starts running the critical section.
    Ideally when first thread has got the lock second should wait, right?
    In other words in a Single CPU Scenario a SpinLock should work like a
    simple Mutex, right?? Or i'm mistaken, it doen't work at all in case of
    Single CPU??
    Or is it like, when one thread has got the Lock, means jumped to
    Dispatch Level, other Threads will simply remain blocked??
    Whatever be the Reason when Thread 1 has got the lock and thread 2 in
    same CPU tries to get the same lock is not blocking untill the first
    Thread releases it?? Am i doing something worng??
    please Help me out, SpinLock is a new topic for me...
    thanks and waiting for reply,
    Rohit
     
    Rohit, Apr 25, 2006
    #1
    1. Advertisements

  2. The purpose of spin locks is to implement a mutex-like object that is
    multiprocessor safe. So this is simply how they are used, i.e. if you
    acquire a spin lock (and thus "own" it) no other thread on whatever CPU
    can also acquire the same spin lock until after the lock is released by
    the current "owner".

    If that does not work for you, you must be doing something wrong, e.g.
    you forgot to initialize the spin lock.

    Or, by any chance, are you running a uniprocessor HAL on a
    multiprocessor system (if that is actually possible at all)?

    Stephan
     
    Stephan Wolf [MVP], Apr 25, 2006
    #2
    1. Advertisements

  3. Rohit

    Don Burn Guest

    Once you have acquired a spin lock you are running at DISPATCH_LEVEL and no
    other thread can run on that CPU, so you are somehow lowering IRQL to below
    DISPATCH_LEVEL, by either KeLowerIrql (or KeRaiseIrql) or by releasing the
    spinlock.
     
    Don Burn, Apr 25, 2006
    #3
  4. Rohit

    Mark Roddy Guest

    That will surely bugcheck with mismatched hal at boot.

    =====================
    Mark Roddy DDK MVP
    Windows Vista/2003/XP/2000 Consulting
    Device and Filesystem Drivers
    Hollis Technology Solutions 603-321-1032
    www.hollistech.com
     
    Mark Roddy, Apr 25, 2006
    #4
  5. Rohit

    Rohit Guest

    My code is as following here i have 3 threads which should one by one,
    but all 3 run parallely:

    PKSPIN_LOCK SpinLock;
    PKIRQL OldIrql;
    static sp_global=0;

    void ixOsalTestSpinLock1(void *arg)
    {
    KeAcquireSpinLock(SpinLock, OldIrql);
    DbgPrint("Inside thread1\n");
    TestSpinLockReentrant("Reentrant function called by Thread 1");
    }

    void ixOsalTestSpinLock2(void *arg)
    {
    KeAcquireSpinLock(SpinLock, OldIrql);
    DbgPrint("Inside thread2\n");
    TestSpinLockReentrant("Reentrant function called by Thread 2");
    }

    void ixOsalTestSpinLock3(void *arg)
    {
    KeAcquireSpinLock(&SpinLock, &OldIrql);
    DbgPrint("Inside thread3\n");
    TestSpinLockReentrant("Reentrant function called by Thread 3");
    }

    void test_spinlock_functions()
    {
    IxOsalThreadAttr attr_sp1, attr_sp2, attr_sp3;
    IxOsalThread id_sp1, id_sp2, id_sp3;

    attr_sp1.priority = 17;
    attr_sp2.priority = 16;
    attr_sp3.priority = 15;

    SpinLock = ixOsalMemAlloc(sizeof(KSPIN_LOCK)); //Internally
    allocates memory
    OldIrql = ixOsalMemAlloc(sizeof(KIRQL)); //Internally allocates
    memory

    KeInitializeSpinLock(SpinLock);

    ixOsalThreadCreate (&id_sp1, &attr_sp1, ixOsalTestSpinLock1
    ,NULL);//internally calls psCreateSystemThread
    ixOsalSleep(500);
    ixOsalThreadCreate (&id_sp2, &attr_sp2, ixOsalTestSpinLock2
    ,NULL);//internally calls psCreateSystemThread
    ixOsalSleep(500);
    ixOsalThreadCreate (&id_sp3, &attr_sp3, ixOsalTestSpinLock3
    ,NULL);//internally calls psCreateSystemThread
    ixOsalSleep(20000); //Sleeps for 20 seconds, internally calls
    keDelayExecutionThread
    //ixOsalMemFree(SpinLock);
    //ixOsalMemFree(OldIrql);
    }

    void TestSpinLockReentrant(char *str)
    {
    DbgPrint("IRQ Level of Reentrant initially = %ld\n",
    (UINT32)KeGetCurrentIrql());
    KeAcquireSpinLock(&SpinLock, &OldIrql);
    DbgPrint("%s\n", str);
    ixOsalSleep(4000);
    KeReleaseSpinLock(&SpinLock, OldIrql);
    DbgPrint("IRQ Level of freed Reentrant = %ld\n",
    (UINT32)KeGetCurrentIrql());
    }
     
    Rohit, Apr 25, 2006
    #5
  6. Rohit

    Don Burn Guest

    You are using one OldIrql globally, the OldIrql variable should be a local
    variable on the stack. I suspect there are other problems but that is the
    obvious first one. For that matter why are you allocating these at all,
    KIRQL and KSPIN_LOCK are both <= sizeof (PULONG) ?
     
    Don Burn, Apr 25, 2006
    #6
  7. Rohit

    Mark Roddy Guest

    Yeah like Callers of KeDelayExecutionThread must be running at IRQL <=
    APC_LEVEL.

    The test is broken. The OP cannot "sleep" with a spinlock held. Driver
    verifier, as always, would have nipped this thread in the bud.
     
    Mark Roddy, Apr 25, 2006
    #7
  8. Rohit

    Skywing Guest

    It doesn't have to be a local on the stack in this implementation, so that
    is not why the OP is having problems. OldIrql is only used as an out
    parameter that is set when the acquire is finished.

    Yes, this may be an implementation detail, but the NDIS spinlock wrapper
    macros that are compiled into every NDIS driver out there will break if this
    ever changes. So, Microsoft is pretty much stuck with it at this point.
    There have been a number of discussions on this subject on the list in the
    past.

    For in-stack queued spinlocks, however, the lock queue handle must not be
    shared between other threads - but the OP is using "plain" spinlocks here.
     
    Skywing, Apr 25, 2006
    #8
  9. You MAY NOT sleep quile holding a spinlock.
     
    Alexander Grigoriev, Apr 25, 2006
    #9
  10. Rohit

    Don Burn Guest

    Yes, technically that is true. And the NDIS implementation has lead to a
    number of problems over the years, there is no reason to encourate people to
    do stupid things that can cause problems when these can be avoided.
     
    Don Burn, Apr 25, 2006
    #10
  11. Rohit

    Skywing Guest

    And as a result, in this case, that is not the cause of the OP's problem
    here. The problem you alluded to is more of a design/philosophical one, but
    it remains true that both ways will work in this scenario, so it's not
    causing the breakage.

    I see the following wrong with the OP's code:

    - in two places the OP passes a KIRQL** and a KSPIN_LOCK** (extra level of
    indirection) to KeAcquireSpinLock - this is broken.
    - the OP attempts to acquire the same spinlock twice on the same callstack -
    this is broken.
     
    Skywing, Apr 25, 2006
    #11
  12. There is no chance of a race condition, his KIRQL is being protected by the
    very spinlock, this is why it doesn't matter if it's local or not. When
    KeReleaseSpinLock is being called it is still being protected. Once you
    realize that you will always know you can use one global KIRQL for each
    KSPINLOCK object. I guess the assumption that KIRQL variables need to be
    local comes forth from the theory that it is unsafe to make any function
    calls while a spinlock is being held.

    /Daniel
     
    Daniel Terhell, Apr 25, 2006
    #12
  13. Rohit

    Don Burn Guest

    No, it comes from people using a global OldIrql for one spinlock, then
    forgetting and creating a second spin lock and not another variable for the
    old Irql. If comes from people who decide to use InStackQueued spin locks,
    and curse as the have to a lot of work to replace the structure with a bunch
    of local variables, and be sure they get it right!

    If you search the archives of a number of the lists, you will see these
    discussions, and see comments by respected Microsoft employees on the
    stupidity of this approach. Yes it will work, but all you are doing is
    creating potential problems for the future. Driver development is hard
    enough without adding problems when you do not have to.
     
    Don Burn, Apr 25, 2006
    #13
  14. Rohit

    Rohit Guest

    Thanks for all those useful comments but i wanted to know that how can
    i wait wait at DISPATCH_LEVEL for few seconds??
    Can i have a for(XYZ; ABC ; OPQ) loop kind of thing?? I mean how can i
    keep a thread busy for 3-4 seconds at DISPATCH_LEVEL??

    And sorry for a mistake, that is at all the places in my code it was
    actually
    KeAcquireSpinLock(SpinLock, OldIrql); & KeReleaseSpinLock(SpinLock,
    *OldIrql);,
    Sorry just a typo mistake...

    But my main concern is waiting for 3-4 seconds at DISPATCH_LEVEL...
    Thanks,
    rohit
     
    Rohit, Apr 26, 2006
    #14
  15. you can't wait at dispatch level. you must run as fast as you can w/out
    waiting. if you need to block, you need to use a passive level lock.

    d
     
    Doron Holan [MS], Apr 26, 2006
    #15
  16. Right. Also, if you actually think you need to wait for some external
    event, this is IMHO usually due to bad design.

    The only way to "wait" fo some event at DISPATCH_LEVEL is by saving the
    context in some way, leave DISPATCH_LEVEL, and come back later when the
    event has actually occured.

    Stephan
     
    Stephan Wolf [MVP], Apr 26, 2006
    #16
  17. Rohit

    Mark Roddy Guest


    Your code is still broken even if TestSpinLockReentrant doesn't call
    ixOsalSleep or even if ixOsalSleep doesn't call
    KeDelayExecutionThread.

    TestSpinLockReentrant reacquires and releases the spinlock. This is
    broken on two levels. First the reacquisition is a bug and will
    deadlock the system on an mp build. On the uni build it will succeed,
    but it is still a bug. Secondly if ixOsalSleep is also doing
    KeAcquireSpinLock/KeReleaseSpinLock (in a loop for example) each
    release allows your other threads to acquire the lock.

    Your original complaint that your multiple threads were all
    concurrently acquiring your spinlock was a result of either calling
    KeDelayExecutionThread or more likely a result of your actually
    releasing the spinlock.
    You can use KeStallExecutionProcessor to busy wait at any IRQL for
    SHORT PERIODS OF TIME. 3-4 seconds is not a short period of time.


    =====================
    Mark Roddy DDK MVP
    Windows Vista/2003/XP/2000 Consulting
    Device and Filesystem Drivers
    Hollis Technology Solutions 603-321-1032
    www.hollistech.com
     
    Mark Roddy, Apr 26, 2006
    #17
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.