Skip to content

Commit

Permalink
Don't Share Previous IRQL for Dispatch RW Locks (#4632) (#4634)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nibanks
nibanks authored Oct 28, 2024
1 parent 9387803 commit f0567e1
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 63 deletions.
16 changes: 8 additions & 8 deletions src/core/binding.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ QuicBindingTraceRundown(
CASTED_CLOG_BYTEARRAY(sizeof(DatapathLocalAddr), &DatapathLocalAddr),
CASTED_CLOG_BYTEARRAY(sizeof(DatapathRemoteAddr), &DatapathRemoteAddr));

CxPlatDispatchRwLockAcquireShared(&Binding->RwLock);
CxPlatDispatchRwLockAcquireShared(&Binding->RwLock, PrevIrql);

for (CXPLAT_LIST_ENTRY* Link = Binding->Listeners.Flink;
Link != &Binding->Listeners;
Expand All @@ -266,7 +266,7 @@ QuicBindingTraceRundown(
CXPLAT_CONTAINING_RECORD(Link, QUIC_LISTENER, Link));
}

CxPlatDispatchRwLockReleaseShared(&Binding->RwLock);
CxPlatDispatchRwLockReleaseShared(&Binding->RwLock, PrevIrql);
}

_IRQL_requires_max_(DISPATCH_LEVEL)
Expand Down Expand Up @@ -327,7 +327,7 @@ QuicBindingRegisterListener(
const BOOLEAN NewWildCard = NewListener->WildCard;
const QUIC_ADDRESS_FAMILY NewFamily = QuicAddrGetFamily(NewAddr);

CxPlatDispatchRwLockAcquireExclusive(&Binding->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&Binding->RwLock, PrevIrql);

//
// For a single binding, listeners are saved in a linked list, sorted by
Expand Down Expand Up @@ -397,7 +397,7 @@ QuicBindingRegisterListener(
}
}

CxPlatDispatchRwLockReleaseExclusive(&Binding->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&Binding->RwLock, PrevIrql);

if (MaximizeLookup &&
!QuicLookupMaximizePartitioning(&Binding->Lookup)) {
Expand Down Expand Up @@ -426,7 +426,7 @@ QuicBindingGetListener(
BOOLEAN FailedAlpnMatch = FALSE;
BOOLEAN FailedAddrMatch = TRUE;

CxPlatDispatchRwLockAcquireShared(&Binding->RwLock);
CxPlatDispatchRwLockAcquireShared(&Binding->RwLock, PrevIrql);

for (CXPLAT_LIST_ENTRY* Link = Binding->Listeners.Flink;
Link != &Binding->Listeners;
Expand Down Expand Up @@ -460,7 +460,7 @@ QuicBindingGetListener(

Done:

CxPlatDispatchRwLockReleaseShared(&Binding->RwLock);
CxPlatDispatchRwLockReleaseShared(&Binding->RwLock, PrevIrql);

if (FailedAddrMatch) {
QuicTraceEvent(
Expand All @@ -487,9 +487,9 @@ QuicBindingUnregisterListener(
_In_ QUIC_LISTENER* Listener
)
{
CxPlatDispatchRwLockAcquireExclusive(&Binding->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&Binding->RwLock, PrevIrql);
CxPlatListEntryRemove(&Listener->Link);
CxPlatDispatchRwLockReleaseExclusive(&Binding->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&Binding->RwLock, PrevIrql);
}

_IRQL_requires_max_(PASSIVE_LEVEL)
Expand Down
56 changes: 28 additions & 28 deletions src/core/lookup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ QuicLookupMaximizePartitioning(
{
BOOLEAN Result = TRUE;

CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock, PrevIrql);

if (!Lookup->MaximizePartitioning) {
Result =
Expand All @@ -263,7 +263,7 @@ QuicLookupMaximizePartitioning(
}
}

CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock, PrevIrql);

return Result;
}
Expand Down Expand Up @@ -369,14 +369,14 @@ QuicLookupFindConnectionByLocalCidInternal(
PartitionIndex %= Lookup->PartitionCount;
QUIC_PARTITIONED_HASHTABLE* Table = &Lookup->HASH.Tables[PartitionIndex];

CxPlatDispatchRwLockAcquireShared(&Table->RwLock);
CxPlatDispatchRwLockAcquireShared(&Table->RwLock, PrevIrql);
Connection =
QuicHashLookupConnection(
&Table->Table,
CID,
CIDLen,
Hash);
CxPlatDispatchRwLockReleaseShared(&Table->RwLock);
CxPlatDispatchRwLockReleaseShared(&Table->RwLock, PrevIrql);
}

#if QUIC_DEBUG_HASHTABLE_LOOKUP
Expand Down Expand Up @@ -487,13 +487,13 @@ QuicLookupInsertLocalCid(
PartitionIndex %= Lookup->PartitionCount;
QUIC_PARTITIONED_HASHTABLE* Table = &Lookup->HASH.Tables[PartitionIndex];

CxPlatDispatchRwLockAcquireExclusive(&Table->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&Table->RwLock, PrevIrql);
CxPlatHashtableInsert(
&Table->Table,
&SourceCid->Entry,
Hash,
NULL);
CxPlatDispatchRwLockReleaseExclusive(&Table->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&Table->RwLock, PrevIrql);
}

if (UpdateRefCount) {
Expand Down Expand Up @@ -617,9 +617,9 @@ QuicLookupRemoveLocalCidInt(
PartitionIndex &= MsQuicLib.PartitionMask;
PartitionIndex %= Lookup->PartitionCount;
QUIC_PARTITIONED_HASHTABLE* Table = &Lookup->HASH.Tables[PartitionIndex];
CxPlatDispatchRwLockAcquireExclusive(&Table->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&Table->RwLock, PrevIrql);
CxPlatHashtableRemove(&Table->Table, &SourceCid->Entry, NULL);
CxPlatDispatchRwLockReleaseExclusive(&Table->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&Table->RwLock, PrevIrql);
}
}

Expand All @@ -634,7 +634,7 @@ QuicLookupFindConnectionByLocalCid(
{
uint32_t Hash = CxPlatHashSimple(CIDLen, CID);

CxPlatDispatchRwLockAcquireShared(&Lookup->RwLock);
CxPlatDispatchRwLockAcquireShared(&Lookup->RwLock, PrevIrql);

QUIC_CONNECTION* ExistingConnection =
QuicLookupFindConnectionByLocalCidInternal(
Expand All @@ -647,7 +647,7 @@ QuicLookupFindConnectionByLocalCid(
QuicConnAddRef(ExistingConnection, QUIC_CONN_REF_LOOKUP_RESULT);
}

CxPlatDispatchRwLockReleaseShared(&Lookup->RwLock);
CxPlatDispatchRwLockReleaseShared(&Lookup->RwLock, PrevIrql);

return ExistingConnection;
}
Expand All @@ -664,7 +664,7 @@ QuicLookupFindConnectionByRemoteHash(
{
uint32_t Hash = QuicPacketHash(RemoteAddress, RemoteCidLength, RemoteCid);

CxPlatDispatchRwLockAcquireShared(&Lookup->RwLock);
CxPlatDispatchRwLockAcquireShared(&Lookup->RwLock, PrevIrql);

QUIC_CONNECTION* ExistingConnection;
if (Lookup->MaximizePartitioning) {
Expand All @@ -684,7 +684,7 @@ QuicLookupFindConnectionByRemoteHash(
ExistingConnection = NULL;
}

CxPlatDispatchRwLockReleaseShared(&Lookup->RwLock);
CxPlatDispatchRwLockReleaseShared(&Lookup->RwLock, PrevIrql);

return ExistingConnection;
}
Expand All @@ -699,7 +699,7 @@ QuicLookupFindConnectionByRemoteAddr(
QUIC_CONNECTION* ExistingConnection = NULL;
UNREFERENCED_PARAMETER(RemoteAddress); // Can't even validate this for single connection lookups right now.

CxPlatDispatchRwLockAcquireShared(&Lookup->RwLock);
CxPlatDispatchRwLockAcquireShared(&Lookup->RwLock, PrevIrql);

if (Lookup->PartitionCount == 0) {
//
Expand All @@ -717,7 +717,7 @@ QuicLookupFindConnectionByRemoteAddr(
QuicConnAddRef(ExistingConnection, QUIC_CONN_REF_LOOKUP_RESULT);
}

CxPlatDispatchRwLockReleaseShared(&Lookup->RwLock);
CxPlatDispatchRwLockReleaseShared(&Lookup->RwLock, PrevIrql);

return ExistingConnection;
}
Expand All @@ -734,7 +734,7 @@ QuicLookupAddLocalCid(
QUIC_CONNECTION* ExistingConnection;
uint32_t Hash = CxPlatHashSimple(SourceCid->CID.Length, SourceCid->CID.Data);

CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock, PrevIrql);

CXPLAT_DBG_ASSERT(!SourceCid->CID.IsInLookupTable);

Expand All @@ -759,7 +759,7 @@ QuicLookupAddLocalCid(
}
}

CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock, PrevIrql);

return Result;
}
Expand All @@ -781,7 +781,7 @@ QuicLookupAddRemoteHash(
QUIC_CONNECTION* ExistingConnection;
uint32_t Hash = QuicPacketHash(RemoteAddress, RemoteCidLength, RemoteCid);

CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock, PrevIrql);

if (Lookup->MaximizePartitioning) {
ExistingConnection =
Expand Down Expand Up @@ -813,7 +813,7 @@ QuicLookupAddRemoteHash(
*Collision = NULL;
}

CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock, PrevIrql);

return Result;
}
Expand All @@ -826,11 +826,11 @@ QuicLookupRemoveLocalCid(
_In_ CXPLAT_SLIST_ENTRY** Entry
)
{
CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock, PrevIrql);
QuicLookupRemoveLocalCidInt(Lookup, SourceCid);
SourceCid->CID.IsInLookupTable = FALSE;
*Entry = (*Entry)->Next;
CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock, PrevIrql);
QuicConnRelease(SourceCid->Connection, QUIC_CONN_REF_LOOKUP_TABLE);
}

Expand All @@ -846,14 +846,14 @@ QuicLookupRemoveRemoteHash(

QuicLibraryOnHandshakeConnectionRemoved();

CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock, PrevIrql);
CXPLAT_DBG_ASSERT(Connection->RemoteHashEntry != NULL);
CxPlatHashtableRemove(
&Lookup->RemoteHashTable,
&RemoteHashEntry->Entry,
NULL);
Connection->RemoteHashEntry = NULL;
CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock, PrevIrql);

CXPLAT_FREE(RemoteHashEntry, QUIC_POOL_REMOTE_HASH);
QuicConnRelease(Connection, QUIC_CONN_REF_LOOKUP_TABLE);
Expand All @@ -868,7 +868,7 @@ QuicLookupRemoveLocalCids(
{
uint8_t ReleaseRefCount = 0;

CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&Lookup->RwLock, PrevIrql);
while (Connection->SourceCids.Next != NULL) {
QUIC_CID_HASH_ENTRY *CID =
CXPLAT_CONTAINING_RECORD(
Expand All @@ -882,7 +882,7 @@ QuicLookupRemoveLocalCids(
}
CXPLAT_FREE(CID, QUIC_POOL_CIDHASH);
}
CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&Lookup->RwLock, PrevIrql);

for (uint8_t i = 0; i < ReleaseRefCount; i++) {
#pragma prefast(suppress:6001, "SAL doesn't understand ref counts")
Expand All @@ -900,7 +900,7 @@ QuicLookupMoveLocalConnectionIDs(
{
CXPLAT_SLIST_ENTRY* Entry = Connection->SourceCids.Next;

CxPlatDispatchRwLockAcquireExclusive(&LookupSrc->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&LookupSrc->RwLock, PrevIrql1);
while (Entry != NULL) {
QUIC_CID_HASH_ENTRY *CID =
CXPLAT_CONTAINING_RECORD(
Expand All @@ -913,9 +913,9 @@ QuicLookupMoveLocalConnectionIDs(
}
Entry = Entry->Next;
}
CxPlatDispatchRwLockReleaseExclusive(&LookupSrc->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&LookupSrc->RwLock, PrevIrql1);

CxPlatDispatchRwLockAcquireExclusive(&LookupDest->RwLock);
CxPlatDispatchRwLockAcquireExclusive(&LookupDest->RwLock, PrevIrql2);
#pragma prefast(suppress:6001, "SAL doesn't understand ref counts")
Entry = Connection->SourceCids.Next;
while (Entry != NULL) {
Expand All @@ -936,5 +936,5 @@ QuicLookupMoveLocalConnectionIDs(
}
Entry = Entry->Next;
}
CxPlatDispatchRwLockReleaseExclusive(&LookupDest->RwLock);
CxPlatDispatchRwLockReleaseExclusive(&LookupDest->RwLock, PrevIrql2);
}
10 changes: 0 additions & 10 deletions src/inc/msquic.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,6 @@ struct CxPlatLockDispatch {
void Acquire() noexcept { CxPlatDispatchLockAcquire(&Handle); }
void Release() noexcept { CxPlatDispatchLockRelease(&Handle); }
};

struct CxPlatRwLockDispatch {
CXPLAT_DISPATCH_RW_LOCK Handle;
CxPlatRwLockDispatch() noexcept { CxPlatDispatchRwLockInitialize(&Handle); }
~CxPlatRwLockDispatch() noexcept { CxPlatDispatchRwLockUninitialize(&Handle); }
void AcquireShared() noexcept { CxPlatDispatchRwLockAcquireShared(&Handle); }
void AcquireExclusive() noexcept { CxPlatDispatchRwLockAcquireExclusive(&Handle); }
void ReleaseShared() noexcept { CxPlatDispatchRwLockReleaseShared(&Handle); }
void ReleaseExclusive() noexcept { CxPlatDispatchRwLockReleaseExclusive(&Handle); }
};
#pragma warning(pop)

struct CxPlatPool {
Expand Down
8 changes: 4 additions & 4 deletions src/inc/quic_platform_posix.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,13 +407,13 @@ typedef CXPLAT_RW_LOCK CXPLAT_DISPATCH_RW_LOCK;

#define CxPlatDispatchRwLockUninitialize CxPlatRwLockUninitialize

#define CxPlatDispatchRwLockAcquireShared CxPlatRwLockAcquireShared
#define CxPlatDispatchRwLockAcquireShared(Lock, PrevIrql) CxPlatRwLockAcquireShared(Lock)

#define CxPlatDispatchRwLockAcquireExclusive CxPlatRwLockAcquireExclusive
#define CxPlatDispatchRwLockAcquireExclusive(Lock, PrevIrql) CxPlatRwLockAcquireExclusive(Lock)

#define CxPlatDispatchRwLockReleaseShared CxPlatRwLockReleaseShared
#define CxPlatDispatchRwLockReleaseShared(Lock, PrevIrql) CxPlatRwLockReleaseShared(Lock)

#define CxPlatDispatchRwLockReleaseExclusive CxPlatRwLockReleaseExclusive
#define CxPlatDispatchRwLockReleaseExclusive(Lock, PrevIrql) CxPlatRwLockReleaseExclusive(Lock)

//
// Represents a QUIC memory pool used for fixed sized allocations.
Expand Down
15 changes: 6 additions & 9 deletions src/inc/quic_platform_winkernel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,17 +314,14 @@ typedef EX_PUSH_LOCK CXPLAT_RW_LOCK;
#define CxPlatRwLockReleaseShared(Lock) ExReleasePushLockShared(Lock); KeLeaveCriticalRegion()
#define CxPlatRwLockReleaseExclusive(Lock) ExReleasePushLockExclusive(Lock); KeLeaveCriticalRegion()

typedef struct CXPLAT_DISPATCH_RW_LOCK {
EX_SPIN_LOCK SpinLock;
KIRQL PrevIrql;
} CXPLAT_DISPATCH_RW_LOCK;
typedef EX_SPIN_LOCK CXPLAT_DISPATCH_RW_LOCK;

#define CxPlatDispatchRwLockInitialize(Lock) (Lock)->SpinLock = 0
#define CxPlatDispatchRwLockInitialize(Lock) *(Lock) = 0
#define CxPlatDispatchRwLockUninitialize(Lock)
#define CxPlatDispatchRwLockAcquireShared(Lock) (Lock)->PrevIrql = ExAcquireSpinLockShared(&(Lock)->SpinLock)
#define CxPlatDispatchRwLockAcquireExclusive(Lock) (Lock)->PrevIrql = ExAcquireSpinLockExclusive(&(Lock)->SpinLock)
#define CxPlatDispatchRwLockReleaseShared(Lock) ExReleaseSpinLockShared(&(Lock)->SpinLock, (Lock)->PrevIrql)
#define CxPlatDispatchRwLockReleaseExclusive(Lock) ExReleaseSpinLockExclusive(&(Lock)->SpinLock, (Lock)->PrevIrql)
#define CxPlatDispatchRwLockAcquireShared(Lock, PrevIrql) KIRQL PrevIrql = ExAcquireSpinLockShared(Lock)
#define CxPlatDispatchRwLockAcquireExclusive(Lock, PrevIrql) KIRQL PrevIrql = ExAcquireSpinLockExclusive(Lock)
#define CxPlatDispatchRwLockReleaseShared(Lock, PrevIrql) ExReleaseSpinLockShared(Lock, PrevIrql)
#define CxPlatDispatchRwLockReleaseExclusive(Lock, PrevIrql) ExReleaseSpinLockExclusive(Lock, PrevIrql)

//
// Reference Count Interface
Expand Down
8 changes: 4 additions & 4 deletions src/inc/quic_platform_winuser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,10 +505,10 @@ typedef SRWLOCK CXPLAT_DISPATCH_RW_LOCK;

#define CxPlatDispatchRwLockInitialize(Lock) InitializeSRWLock(Lock)
#define CxPlatDispatchRwLockUninitialize(Lock)
#define CxPlatDispatchRwLockAcquireShared(Lock) AcquireSRWLockShared(Lock)
#define CxPlatDispatchRwLockAcquireExclusive(Lock) AcquireSRWLockExclusive(Lock)
#define CxPlatDispatchRwLockReleaseShared(Lock) ReleaseSRWLockShared(Lock)
#define CxPlatDispatchRwLockReleaseExclusive(Lock) ReleaseSRWLockExclusive(Lock)
#define CxPlatDispatchRwLockAcquireShared(Lock, PrevIrql) AcquireSRWLockShared(Lock)
#define CxPlatDispatchRwLockAcquireExclusive(Lock, PrevIrql) AcquireSRWLockExclusive(Lock)
#define CxPlatDispatchRwLockReleaseShared(Lock, PrevIrql) ReleaseSRWLockShared(Lock)
#define CxPlatDispatchRwLockReleaseExclusive(Lock, PrevIrql) ReleaseSRWLockExclusive(Lock)

//
// Reference Count Interface
Expand Down

0 comments on commit f0567e1

Please sign in to comment.