To answer Ged's worry, I don't think here people would get angry, since that spinlock is also used there: https://git.reactos.org/?p=reactos.git&a=search&h=HEAD&st=grep&s=LockQueueIoDatabaseLock (see iomgr/file.c and volume.c) .
-----Message d'origine----- De : Ros-dev [mailto:ros-dev-boun...@reactos.org] De la part de Thomas Faber Envoyé : mardi 29 décembre 2015 22:07 À : ReactOS Development List 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 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:ros-dev-boun...@reactos.org] On Behalf Of Hermès > BÉLUSCA - MAÏTO > Sent: 29 December 2015 16:08 > To: 'ReactOS Development List' <ros-dev@reactos.org> > 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:ros-dev-boun...@reactos.org] De la part de Thomas > Faber Envoyé : mardi 29 décembre 2015 13:20 À : ros-dev@reactos.org 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, gedmur...@svn.reactos.org 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/de >> v 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 Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev _______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
