On Sat, Feb 13, 2021 at 10:26:48AM +0100, Marcus Glocker wrote: > On Sat, Feb 13, 2021 at 08:30:04AM +0100, Claudio Jeker wrote: > > > On Fri, Feb 12, 2021 at 10:59:05PM +0100, Jeremie Courreges-Anglas wrote: > > > On Wed, Feb 10 2021, Martin Pieuchot <m...@openbsd.org> wrote: > > > > > > [...] > > > > > > > Which fields is the new lock protecting? Why isn't the KERNEL_LOCK() > > > > enough? > > > > > > When I mentioned this potential lack of locking to Marcus, I was > > > mistaken. Some of the functions in video.c are called from syscalls > > > that are marked NOLOCK. But now that I have added some printf > > > debugging, I can see that the kernel lock is held in places where > > > I didn't expect it to be held (videoioctl, videoread, videoclose...). > > > So there's probably a layer of locking that I am missing. > > > > > > Marcus, sorry for my misleading diff. O:) > > > > > > *crawls back under his rock* > > > > Just because a syscall is marked NOLOCK does not mean that all of it will > > run without the KERNEL_LOCK. For example read and write are marked NOLOCK > > but the KERNEL_LOCK is grabbed later for all non-socket filedescriptors. > > This is why video(4) still runs with the KERNEL_LOCK. > > The NOLOCK in syscalls.master just tell the kernel to not grab the > > KERNEL_LOCK right at the start of the syscall. > > OK, that wasn't clear to me either! If Jeremie has some space left > under his rock, I would join :-) > > That basically means we need no extra locking in video(4) for this > implementation, right? For a test I replaced the rw_enter_write() > with KERNEL_ASSERT_LOCKED(), running a stream and multiple concurrent > ioctl programs, and all went fine. > > I would come up with a revised diff then for the device owner part. > Just doing some polishing together with Anton on that.
For now this is OK. I have some diffs around which will allow unlocked read and write down to individual devices (I was experimenting with tun(4) and tap(4)). For ioctl() this will not happen any time soon. ioctl() is a world of pain. Maybe for devices like video(4) the call path will be manageable but for network devices it is just mess. The main issue is that ioctls work on multiple layers and sometimes the calls have layer violations and often multiple sleep points built into them. -- :wq Claudio
