From 0801068a35de140186919523bf13a346af4c422b Mon Sep 17 00:00:00 2001 From: Johannes Anderwald Date: Fri, 14 May 2010 15:47:00 +0000 Subject: [PATCH] [PORTCLS] - Don't request initializing delayed service request as this is the task of the miniport driver - Reimplement the service group object: - Use the initialized timer object when RequestService is called - Fix possible race conditions when adding / removing a service sink by protecting it with a lock - Acquire the service group list lock when executing the shared dpc routine svn path=/trunk/; revision=47197 --- .../wdm/audio/backpln/portcls/pin_dmus.cpp | 1 - .../audio/backpln/portcls/pin_wavecyclic.cpp | 3 +- .../wdm/audio/backpln/portcls/pin_wavepci.cpp | 1 - .../audio/backpln/portcls/service_group.cpp | 192 +++++++++++------- 4 files changed, 116 insertions(+), 81 deletions(-) diff --git a/reactos/drivers/wdm/audio/backpln/portcls/pin_dmus.cpp b/reactos/drivers/wdm/audio/backpln/portcls/pin_dmus.cpp index da6e98d9d51..991e0e11426 100644 --- a/reactos/drivers/wdm/audio/backpln/portcls/pin_dmus.cpp +++ b/reactos/drivers/wdm/audio/backpln/portcls/pin_dmus.cpp @@ -602,7 +602,6 @@ CPortPinDMus::Init( DPRINT("Failed to add pin to service group\n"); return Status; } - m_ServiceGroup->SupportDelayedService(); } Status = m_IrpQueue->Init(ConnectDetails, 0, 0, NULL); diff --git a/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp b/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp index ff02905e386..0520480ae92 100644 --- a/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp +++ b/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp @@ -1213,7 +1213,6 @@ CPortPinWaveCyclic::Init( return Status; } - m_ServiceGroup->SupportDelayedService(); m_Stream->SetState(KSSTATE_STOP); m_State = KSSTATE_STOP; m_CommonBufferOffset = 0; @@ -1224,6 +1223,8 @@ CPortPinWaveCyclic::Init( m_Delay = Int32x32To64(10, -10000); Status = m_Stream->SetNotificationFreq(10, &m_FrameSize); + PC_ASSERT(NT_SUCCESS(Status)); + PC_ASSERT(m_FrameSize); SilenceBuffer = AllocateItem(NonPagedPool, m_FrameSize, TAG_PORTCLASS); if (!SilenceBuffer) diff --git a/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp b/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp index 935ece83ec7..1b44fe1794f 100644 --- a/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp +++ b/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp @@ -815,7 +815,6 @@ CPortPinWavePci::Init( DPRINT("Failed to add pin to service group\n"); return Status; } - m_ServiceGroup->SupportDelayedService(); } // delay of 10 milisec diff --git a/reactos/drivers/wdm/audio/backpln/portcls/service_group.cpp b/reactos/drivers/wdm/audio/backpln/portcls/service_group.cpp index 8e2bb943cf8..3a17faeec85 100644 --- a/reactos/drivers/wdm/audio/backpln/portcls/service_group.cpp +++ b/reactos/drivers/wdm/audio/backpln/portcls/service_group.cpp @@ -54,12 +54,10 @@ protected: LIST_ENTRY m_ServiceSinkHead; - BOOL m_Initialized; - BOOL m_TimerActive; + BOOL m_TimerInitialized; KTIMER m_Timer; KDPC m_Dpc; - KEVENT m_Event; - LONG m_ThreadActive; + KSPIN_LOCK m_Lock; friend VOID NTAPI IServiceGroupDpc(IN struct _KDPC *Dpc, IN PVOID DeferredContext, IN PVOID SystemArgument1, IN PVOID SystemArgument2); @@ -105,9 +103,16 @@ CServiceGroup::QueryInterface( CServiceGroup::CServiceGroup(IUnknown * OuterUnknown) { + // initialize dpc KeInitializeDpc(&m_Dpc, IServiceGroupDpc, (PVOID)this); + + // set highest importance KeSetImportanceDpc(&m_Dpc, HighImportance); - KeInitializeEvent(&m_Event, NotificationEvent, FALSE); + + // initialize service group list lock + KeInitializeSpinLock(&m_Lock); + + // initialize service group list InitializeListHead(&m_ServiceSinkHead); } @@ -119,15 +124,34 @@ CServiceGroup::RequestService() DPRINT("CServiceGroup::RequestService() Dpc at Level %u\n", KeGetCurrentIrql()); - if (KeGetCurrentIrql() > DISPATCH_LEVEL) + if (m_TimerInitialized) { - KeInsertQueueDpc(&m_Dpc, NULL, NULL); - return; - } + LARGE_INTEGER DueTime; - KeRaiseIrql(DISPATCH_LEVEL, &OldIrql); - KeInsertQueueDpc(&m_Dpc, NULL, NULL); - KeLowerIrql(OldIrql); + // no due time + DueTime.QuadPart = 0LL; + + // delayed service requested + KeSetTimer(&m_Timer, DueTime, &m_Dpc); + } + else + { + // check curent irql + if (KeGetCurrentIrql() > DISPATCH_LEVEL) + { + //insert dpc to queue + KeInsertQueueDpc(&m_Dpc, NULL, NULL); + } + else + { + // raise irql to dispatch level to make dpc fire immediately + KeRaiseIrql(DISPATCH_LEVEL, &OldIrql); + // insert dpc to queue + KeInsertQueueDpc(&m_Dpc, NULL, NULL); + // lower irql to old level + KeLowerIrql(OldIrql); + } + } } //--------------------------------------------------------------- @@ -140,18 +164,33 @@ CServiceGroup::AddMember( IN PSERVICESINK pServiceSink) { PGROUP_ENTRY Entry; + KIRQL OldLevel; + // sanity check PC_ASSERT_IRQL_EQUAL(PASSIVE_LEVEL); + // allocate service sink entry Entry = (PGROUP_ENTRY)AllocateItem(NonPagedPool, sizeof(GROUP_ENTRY), TAG_PORTCLASS); if (!Entry) + { + // out of memory return STATUS_INSUFFICIENT_RESOURCES; + } + // initialize service sink entry Entry->pServiceSink = pServiceSink; + // increment reference count pServiceSink->AddRef(); + // acquire service group list lock + KeAcquireSpinLock(&m_Lock, &OldLevel); + + // insert into service sink list InsertTailList(&m_ServiceSinkHead, &Entry->Entry); + // release service group list lock + KeReleaseSpinLock(&m_Lock, OldLevel); + return STATUS_SUCCESS; } @@ -162,23 +201,45 @@ CServiceGroup::RemoveMember( { PLIST_ENTRY CurEntry; PGROUP_ENTRY Entry; + KIRQL OldLevel; + // sanity check PC_ASSERT_IRQL_EQUAL(PASSIVE_LEVEL); + // acquire service group list lock + KeAcquireSpinLock(&m_Lock, &OldLevel); + + // grab first entry CurEntry = m_ServiceSinkHead.Flink; + + // loop list until the passed entry is found while (CurEntry != &m_ServiceSinkHead) { + // grab entry Entry = CONTAINING_RECORD(CurEntry, GROUP_ENTRY, Entry); + + // check if it matches the passed entry if (Entry->pServiceSink == pServiceSink) { + // remove entry from list RemoveEntryList(&Entry->Entry); + + // release service sink reference pServiceSink->Release(); + + // free service sink entry FreeItem(Entry, TAG_PORTCLASS); - return; + + // leave loop + break; } + // move to next entry CurEntry = CurEntry->Flink; } + // release service group list lock + KeReleaseSpinLock(&m_Lock, OldLevel); + } VOID @@ -194,73 +255,40 @@ IServiceGroupDpc( PGROUP_ENTRY Entry; CServiceGroup * This = (CServiceGroup*)DeferredContext; + // acquire service group list lock + KeAcquireSpinLockAtDpcLevel(&This->m_Lock); + + // grab first entry CurEntry = This->m_ServiceSinkHead.Flink; + + // loop the list and call the attached service sink/group while (CurEntry != &This->m_ServiceSinkHead) { + //grab current entry Entry = (PGROUP_ENTRY)CONTAINING_RECORD(CurEntry, GROUP_ENTRY, Entry); + + // call service sink/group Entry->pServiceSink->RequestService(); + + // move to next entry CurEntry = CurEntry->Flink; } + + // release service group list lock + KeReleaseSpinLockFromDpcLevel(&This->m_Lock); } - -#if 0 -VOID -NTAPI -ServiceGroupThread(IN PVOID StartContext) -{ - NTSTATUS Status; - KWAIT_BLOCK WaitBlockArray[2]; - PVOID WaitObjects[2]; - CServiceGroup * This = (CServiceGroup*)StartContext; - - // Set thread state - InterlockedIncrement(&This->m_ThreadActive); - - // Setup the wait objects - WaitObjects[0] = &m_Timer; - WaitObjects[1] = &m_Event; - - do - { - // Wait on our objects - Status = KeWaitForMultipleObjects(2, WaitObjects, WaitAny, Executive, KernelMode, FALSE, NULL, WaitBlockArray); - - switch(Status) - { - case STATUS_WAIT_0: - IServiceGroupDpc(&This->m_Dpc, (PVOID)This, NULL, NULL); - break; - case STATUS_WAIT_1: - PsTerminateSystemThread(STATUS_SUCCESS); - return; - } - }while(TRUE); -} - -#endif VOID NTAPI CServiceGroup::SupportDelayedService() { - //NTSTATUS Status; - //HANDLE ThreadHandle; - PC_ASSERT_IRQL(DISPATCH_LEVEL); - if (m_Initialized) - return; + // initialize the timer + KeInitializeTimer(&m_Timer); - KeInitializeTimerEx(&m_Timer, NotificationTimer); - -#if 0 - Status = PsCreateSystemThread(&ThreadHandle, THREAD_ALL_ACCESS, NULL, 0, NULL, ServiceGroupThread, (PVOID)This); - if (NT_SUCCESS(Status)) - { - ZwClose(ThreadHandle); - m_Initialized = TRUE; - } -#endif + // use the timer to perform service requests + m_TimerInitialized = TRUE; } VOID @@ -270,17 +298,14 @@ CServiceGroup::RequestDelayedService( { LARGE_INTEGER DueTime; + // sanity check PC_ASSERT_IRQL(DISPATCH_LEVEL); + PC_ASSERT(m_TimerInitialized); DueTime.QuadPart = ullDelay; - if (m_Initialized) - { - if (KeGetCurrentIrql() <= DISPATCH_LEVEL) - KeSetTimer(&m_Timer, DueTime, &m_Dpc); - else - KeInsertQueueDpc(&m_Dpc, NULL, NULL); - } + // set the timer + KeSetTimer(&m_Timer, DueTime, &m_Dpc); } VOID @@ -288,11 +313,10 @@ NTAPI CServiceGroup::CancelDelayedService() { PC_ASSERT_IRQL(DISPATCH_LEVEL); + PC_ASSERT(m_TimerInitialized); - if (m_Initialized) - { - KeCancelTimer(&m_Timer); - } + // cancel the timer + KeCancelTimer(&m_Timer); } NTSTATUS @@ -303,19 +327,31 @@ PcNewServiceGroup( { CServiceGroup * This; NTSTATUS Status; + DPRINT("PcNewServiceGroup entered\n"); - This = new(NonPagedPool, TAG_PORTCLASS)CServiceGroup(OuterUnknown); - if (!This) - return STATUS_INSUFFICIENT_RESOURCES; + //FIXME support aggregation + PC_ASSERT(OuterUnknown == NULL); + // allocate a service group object + This = new(NonPagedPool, TAG_PORTCLASS)CServiceGroup(OuterUnknown); + + if (!This) + { + // out of memory + return STATUS_INSUFFICIENT_RESOURCES; + } + + // request IServiceSink interface Status = This->QueryInterface(IID_IServiceSink, (PVOID*)OutServiceGroup); if (!NT_SUCCESS(Status)) { + // failed to acquire service sink interface delete This; return Status; } + // done return Status; }