Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't Share Previous IRQL for Dispatch RW Locks (#4632) #4634

Merged
merged 1 commit into from
Oct 28, 2024
Merged

Don't Share Previous IRQL for Dispatch RW Locks (#4632) #4634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/core/binding.c
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading