I think this might be an SP/MP difference? Would make sense if KeAcquireSpinLock inside an SP kernel becomes KeRaiseIrql. But even for a 2003 MP kernel this would serve no purpose.
On 2015-12-29 21:48, Ged Murphy wrote: > It depends on whether we want to emulate the 2k3 kernel or not. > > I noticed in my 2k3 fltmgr testing that IoEnumerateDeviceObjectList runs at > DPC for the entirety of the function. This was changed in the NT6 kernel to > use a lock, which is indeed the LockQueueIoDatabaseLock Hermes mentioned. > > I'm happy to use the spinlock (and would prefer to myself), but any move > towards NT6 normally results in an angry email, so I opted for the 2k3 > approach ;) > > Ged. > > > -----Original Message----- > From: Ros-dev [mailto:[email protected]] On Behalf Of Hermès > BÉLUSCA - MAÏTO > Sent: 29 December 2015 16:08 > To: 'ReactOS Development List' <[email protected]> > Subject: Re: [ros-dev] [ros-diffs] [gedmurphy] 70408: [NTOSKRNL] - Raise the > IRQL when enumerating device lists so it doesn't get edited mid-listing - > Don't hardcode the pointer size when checking the buffer size > > Yes this should be a spinlock involved instead, for example the > "LockQueueIoDatabaseLock" (queued) spinlock that we already use in other > places in the code. > > -----Message d'origine----- > De : Ros-dev [mailto:[email protected]] De la part de Thomas Faber > Envoyé : mardi 29 décembre 2015 13:20 À : [email protected] Objet : Re: > [ros-dev] [ros-diffs] [gedmurphy] 70408: [NTOSKRNL] - Raise the IRQL when > enumerating device lists so it doesn't get edited mid-listing - Don't > hardcode the pointer size when checking the buffer size > > Uhm... raising the IRQL is not a synchronization mechanism. Should there be > a spinlock involved? > > > On 2015-12-23 12:26, [email protected] wrote: >> Author: gedmurphy >> Date: Wed Dec 23 11:26:28 2015 >> New Revision: 70408 >> >> URL: http://svn.reactos.org/svn/reactos?rev=70408&view=rev >> Log: >> [NTOSKRNL] >> - Raise the IRQL when enumerating device lists so it doesn't get >> edited mid-listing >> - Don't hardcode the pointer size when checking the buffer size >> >> Modified: >> trunk/reactos/ntoskrnl/io/iomgr/device.c >> >> Modified: trunk/reactos/ntoskrnl/io/iomgr/device.c >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/dev >> ice.c?rev=70408&r1=70407&r2=70408&view=diff >> > ============================================================================ > == >> --- trunk/reactos/ntoskrnl/io/iomgr/device.c [iso-8859-1] (original) >> +++ trunk/reactos/ntoskrnl/io/iomgr/device.c [iso-8859-1] Wed Dec 23 > 11:26:28 2015 >> @@ -1088,6 +1088,10 @@ >> { >> ULONG ActualDevices = 1; >> PDEVICE_OBJECT CurrentDevice = DriverObject->DeviceObject; >> + KIRQL OldIrql; >> + >> + /* Raise to dispatch level */ >> + KeRaiseIrql(DISPATCH_LEVEL, &OldIrql); >> >> /* Find out how many devices we'll enumerate */ >> while ((CurrentDevice = CurrentDevice->NextDevice)) >> ActualDevices++; @@ -1099,13 +1103,14 @@ >> *ActualNumberDeviceObjects = ActualDevices; >> >> /* Check if we can support so many */ >> - if ((ActualDevices * 4) > DeviceObjectListSize) >> + if ((ActualDevices * sizeof(PDEVICE_OBJECT)) > >> + DeviceObjectListSize) >> { >> /* Fail because the buffer was too small */ >> + KeLowerIrql(OldIrql); >> return STATUS_BUFFER_TOO_SMALL; >> } >> >> - /* Check if the caller only wanted the size */ >> + /* Check if the caller wanted the device list */ >> if (DeviceObjectList) >> { >> /* Loop through all the devices */ @@ -1123,6 +1128,9 @@ >> DeviceObjectList++; >> } >> } >> + >> + /* Return back to previous IRQL */ >> + KeLowerIrql(OldIrql); >> >> /* Return the status */ >> return STATUS_SUCCESS; >> >> _______________________________________________ Ros-dev mailing list [email protected] http://www.reactos.org/mailman/listinfo/ros-dev
