On 16/06/18(Sat) 19:28, Mark Kettenis wrote: > > Date: Thu, 14 Jun 2018 14:28:07 +0200 > > From: Martin Pieuchot <m...@openbsd.org> > > > > This version should fixes the previous issue where LARVAL files were > > incorrectly accounted in find_last_set() resulting in a panic() in > > fd_unused(). > > > > If you haven't read the previous explanations, the idea of this diff > > is to publish file descriptors only when they are completely setup. > > This will allow us to serialize access to file descriptors in a mp-safe > > way, which is the last requirement for unlock most of the socket-related > > syscalls. > > > > The important point of this diff is that we don't store on the descriptor > > itself the fact that it isn't ready yet (LARVAL). Otherwise we have a > > chicken & egg problem for reference counting. As pointed by Mathieu > > Masson other BSDs do that by using a new structure for open files. We > > might want to do that later when we'll turn these functions mp-safe :) > > For now, using the existing bitmask is good enough. > > > > Here are previous explanations: > > https://marc.info/?l=openbsd-tech&m=152579630904328&w=2 > > https://marc.info/?l=openbsd-tech&m=152750590114899&w=2 > > > > Please test & report as usual. > > > > Ok? > > Sorry, but no, I don't think this diff is ok. Your new code inserts > the struct file into the linked list of all open files on fdinsert().
Yes, that's the goal of my diff. No descriptor should be reachable by another thread before fdinsert() has been called. Since the linked list offers such possibility it should be delayed. > However, there is code that bails out and calls closef() before that > fdinsert() happens. For exacmple in kern_exec.c:sys_execve() and in > sys_pipe.c:dopipe() (if the 2nd falloc() call fails). There's no problem with that. > I really think falloc() should continue to insert the struct file in > the list. That list is only used for userland tools like fstat and > fuser. And those tools should handle open files that don't have a > file descriptor. Such files almost never exist Mark, only in accept(2). All other sycalls block on the fdplock(). I'm not going to change the sysctl interface to make it MP-safe and fix all the related bug in all subsystems ;) However I'd glad if you could come up with a way to show such files in a MP-safe way after I've committed my diff. You can even try visa@'s diff: https://marc.info/?l=openbsd-tech&m=152794586504816&w=2
