On Tue, 8 May 2018, Martin Pieuchot wrote: > The way our kernel allocates and populates new 'struct file *' is > currently a complete mess. One of the problems is that it puts > incomplete descriptors on the global `filehead' list and on the > open file array `fd_ofiles'. But to make sure that other threads > won't use such descriptors before they are ready, they are first > marked as LARVAL then unmarked via the FILE_SET_MATURE() macro.
The reason we do that is to meet two goals: 1) we want to allocate the fd (and fail if we can't) before possibly blocking while completing the creation of the struct file. For example, in doaccept(), this thread being blocked waiting for a new connection must not block other threads from making fd changes. 2) close(half_created) and dup2(old, half_created) must be safe: it must not leak the half_created file in the kernel, and on completion the half_created fd must either be closed or dup'ed to; the half-created file must not suddenly show up there later > This is a problem because we'll soon need to serialize accesses > to the above mentioned data structures with finer locks. And having > to deal with specific checks in multiple places is simply not viable. > > So the diff below changes the logic by introducing a new function: > fdinsert(). This function will be used to insert new descriptors into > shared data structured when they are completely initialized. doaccept() is still releasing the filedesc lock between falloc() and what used to be FILE_SET_MATURE() but is now fdinsert(), but that's not safe since the fd_ofile[] entry is still NULL so finishdup() and close() will happily overwrite it or return and then later doaccept()->fdinsert() will overwrite one pointer with another, leaking file references. So this diff is not safe. Philip
