Bläddra i källkod

Kernel: Switch process file descriptor table from spinlock to mutex

There's no reason for this to use a spinlock. Instead, let's allow
threads to block if someone else is using the descriptor table.
Andreas Kling 3 år sedan
förälder
incheckning
b56646e293

+ 1 - 1
Kernel/GlobalProcessExposed.cpp

@@ -490,7 +490,7 @@ private:
             process_object.add("uid", process.uid().value());
             process_object.add("gid", process.gid().value());
             process_object.add("ppid", process.ppid().value());
-            process_object.add("nfds", process.fds().with([](auto& fds) { return fds.open_count(); }));
+            process_object.add("nfds", process.fds().with_shared([](auto& fds) { return fds.open_count(); }));
             process_object.add("name", process.name());
             process_object.add("executable", process.executable() ? TRY(process.executable()->try_serialize_absolute_path())->view() : ""sv);
             process_object.add("tty", process.tty() ? process.tty()->tty_name().view() : "notty"sv);

+ 2 - 2
Kernel/Process.cpp

@@ -139,7 +139,7 @@ ErrorOr<NonnullRefPtr<Process>> Process::try_create_user_process(RefPtr<Thread>&
     auto name = TRY(KString::try_create(parts.last()));
     auto process = TRY(Process::try_create(first_thread, move(name), uid, gid, ProcessID(0), false, VirtualFileSystem::the().root_custody(), nullptr, tty));
 
-    TRY(process->m_fds.with([&](auto& fds) -> ErrorOr<void> {
+    TRY(process->m_fds.with_exclusive([&](auto& fds) -> ErrorOr<void> {
         TRY(fds.try_resize(Process::OpenFileDescriptions::max_open()));
 
         auto& device_to_use_as_tty = tty ? (CharacterDevice&)*tty : DeviceManagement::the().null_device();
@@ -589,7 +589,7 @@ void Process::finalize()
 
     if (m_alarm_timer)
         TimerQueue::the().cancel_timer(m_alarm_timer.release_nonnull());
-    m_fds.with([](auto& fds) { fds.clear(); });
+    m_fds.with_exclusive([](auto& fds) { fds.clear(); });
     m_tty = nullptr;
     m_executable = nullptr;
     m_cwd = nullptr;

+ 6 - 6
Kernel/Process.h

@@ -746,22 +746,22 @@ public:
         WeakPtr<Process> m_process;
     };
 
-    SpinlockProtected<OpenFileDescriptions>& fds() { return m_fds; }
-    SpinlockProtected<OpenFileDescriptions> const& fds() const { return m_fds; }
+    MutexProtected<OpenFileDescriptions>& fds() { return m_fds; }
+    MutexProtected<OpenFileDescriptions> const& fds() const { return m_fds; }
 
     ErrorOr<NonnullRefPtr<OpenFileDescription>> open_file_description(int fd)
     {
-        return m_fds.with([fd](auto& fds) { return fds.open_file_description(fd); });
+        return m_fds.with_shared([fd](auto& fds) { return fds.open_file_description(fd); });
     }
 
     ErrorOr<NonnullRefPtr<OpenFileDescription>> open_file_description(int fd) const
     {
-        return m_fds.with([fd](auto& fds) { return fds.open_file_description(fd); });
+        return m_fds.with_shared([fd](auto& fds) { return fds.open_file_description(fd); });
     }
 
     ErrorOr<ScopedDescriptionAllocation> allocate_fd()
     {
-        return m_fds.with([](auto& fds) { return fds.allocate(); });
+        return m_fds.with_exclusive([](auto& fds) { return fds.allocate(); });
     }
 
 private:
@@ -770,7 +770,7 @@ private:
 
     SpinlockProtected<Thread::ListInProcess> m_thread_list;
 
-    SpinlockProtected<OpenFileDescriptions> m_fds;
+    MutexProtected<OpenFileDescriptions> m_fds;
 
     const bool m_is_kernel_process;
     Atomic<State> m_state { State::Running };

+ 3 - 3
Kernel/ProcessSpecificExposed.cpp

@@ -95,7 +95,7 @@ ErrorOr<void> Process::traverse_file_descriptions_directory(FileSystemID fsid, F
     TRY(callback({ ".", { fsid, m_procfs_traits->component_index() }, 0 }));
     TRY(callback({ "..", { fsid, m_procfs_traits->component_index() }, 0 }));
     size_t count = 0;
-    fds().with([&](auto& fds) {
+    fds().with_shared([&](auto& fds) {
         fds.enumerate([&](auto& file_description_metadata) {
             if (!file_description_metadata.is_valid()) {
                 count++;
@@ -117,7 +117,7 @@ ErrorOr<NonnullRefPtr<Inode>> Process::lookup_file_descriptions_directory(const
     if (!maybe_index.has_value())
         return ENOENT;
 
-    if (!fds().with([&](auto& fds) { return fds.get_if_valid(*maybe_index); }))
+    if (!m_fds.with_shared([&](auto& fds) { return fds.get_if_valid(*maybe_index); }))
         return ENOENT;
 
     return TRY(ProcFSProcessPropertyInode::try_create_for_file_description_link(procfs, *maybe_index, pid()));
@@ -181,7 +181,7 @@ ErrorOr<void> Process::procfs_get_fds_stats(KBufferBuilder& builder) const
 {
     JsonArraySerializer array { builder };
 
-    return fds().with([&](auto& fds) -> ErrorOr<void> {
+    return fds().with_shared([&](auto& fds) -> ErrorOr<void> {
         if (fds.open_count() == 0) {
             array.finish();
             return {};

+ 1 - 1
Kernel/Syscalls/anon_create.cpp

@@ -37,7 +37,7 @@ ErrorOr<FlatPtr> Process::sys$anon_create(size_t size, int options)
     if (options & O_CLOEXEC)
         fd_flags |= FD_CLOEXEC;
 
-    m_fds.with([&](auto& fds) { fds[new_fd.fd].set(move(description), fd_flags); });
+    m_fds.with_exclusive([&](auto& fds) { fds[new_fd.fd].set(move(description), fd_flags); });
     return new_fd.fd;
 }
 

+ 1 - 1
Kernel/Syscalls/dup2.cpp

@@ -13,7 +13,7 @@ ErrorOr<FlatPtr> Process::sys$dup2(int old_fd, int new_fd)
 {
     VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this);
     TRY(require_promise(Pledge::stdio));
-    return m_fds.with([&](auto& fds) -> ErrorOr<FlatPtr> {
+    return m_fds.with_exclusive([&](auto& fds) -> ErrorOr<FlatPtr> {
         auto description = TRY(fds.open_file_description(old_fd));
         if (old_fd == new_fd)
             return new_fd;

+ 2 - 2
Kernel/Syscalls/execve.cpp

@@ -536,7 +536,7 @@ ErrorOr<void> Process::do_exec(NonnullRefPtr<OpenFileDescription> main_program_d
 
     clear_futex_queues_on_exec();
 
-    m_fds.with([&](auto& fds) {
+    m_fds.with_exclusive([&](auto& fds) {
         fds.change_each([&](auto& file_description_metadata) {
             if (file_description_metadata.is_valid() && file_description_metadata.flags() & FD_CLOEXEC)
                 file_description_metadata = {};
@@ -545,7 +545,7 @@ ErrorOr<void> Process::do_exec(NonnullRefPtr<OpenFileDescription> main_program_d
 
     if (main_program_fd_allocation.has_value()) {
         main_program_description->set_readable(true);
-        m_fds.with([&](auto& fds) { fds[main_program_fd_allocation->fd].set(move(main_program_description), FD_CLOEXEC); });
+        m_fds.with_exclusive([&](auto& fds) { fds[main_program_fd_allocation->fd].set(move(main_program_description), FD_CLOEXEC); });
     }
 
     new_main_thread = nullptr;

+ 3 - 3
Kernel/Syscalls/fcntl.cpp

@@ -23,16 +23,16 @@ ErrorOr<FlatPtr> Process::sys$fcntl(int fd, int cmd, u32 arg)
         int arg_fd = (int)arg;
         if (arg_fd < 0)
             return EINVAL;
-        return m_fds.with([&](auto& fds) -> ErrorOr<FlatPtr> {
+        return m_fds.with_exclusive([&](auto& fds) -> ErrorOr<FlatPtr> {
             auto fd_allocation = TRY(fds.allocate(arg_fd));
             fds[fd_allocation.fd].set(*description);
             return fd_allocation.fd;
         });
     }
     case F_GETFD:
-        return m_fds.with([fd](auto& fds) { return fds[fd].flags(); });
+        return m_fds.with_exclusive([fd](auto& fds) { return fds[fd].flags(); });
     case F_SETFD:
-        m_fds.with([fd, arg](auto& fds) { fds[fd].set_flags(arg); });
+        m_fds.with_exclusive([fd, arg](auto& fds) { fds[fd].set_flags(arg); });
         break;
     case F_GETFL:
         return description->file_flags();

+ 2 - 2
Kernel/Syscalls/fork.cpp

@@ -23,8 +23,8 @@ ErrorOr<FlatPtr> Process::sys$fork(RegisterState& regs)
     child->m_veil_state = m_veil_state;
     child->m_unveiled_paths = m_unveiled_paths.deep_copy();
 
-    TRY(child->m_fds.with([&](auto& child_fds) {
-        return m_fds.with([&](auto& parent_fds) {
+    TRY(child->m_fds.with_exclusive([&](auto& child_fds) {
+        return m_fds.with_exclusive([&](auto& parent_fds) {
             return child_fds.try_clone(parent_fds);
         });
     }));

+ 1 - 1
Kernel/Syscalls/inode_watcher.cpp

@@ -26,7 +26,7 @@ ErrorOr<FlatPtr> Process::sys$create_inode_watcher(u32 flags)
     if (flags & static_cast<unsigned>(InodeWatcherFlags::Nonblock))
         description->set_blocking(false);
 
-    return m_fds.with([&](auto& fds) -> ErrorOr<FlatPtr> {
+    return m_fds.with_exclusive([&](auto& fds) -> ErrorOr<FlatPtr> {
         fds[fd_allocation.fd].set(move(description));
 
         if (flags & static_cast<unsigned>(InodeWatcherFlags::CloseOnExec))

+ 2 - 2
Kernel/Syscalls/open.cpp

@@ -59,7 +59,7 @@ ErrorOr<FlatPtr> Process::sys$open(Userspace<const Syscall::SC_open_params*> use
     if (description->inode() && description->inode()->socket())
         return ENXIO;
 
-    return m_fds.with([&](auto& fds) -> ErrorOr<FlatPtr> {
+    return m_fds.with_exclusive([&](auto& fds) -> ErrorOr<FlatPtr> {
         u32 fd_flags = (options & O_CLOEXEC) ? FD_CLOEXEC : 0;
         fds[fd_allocation.fd].set(move(description), fd_flags);
         return fd_allocation.fd;
@@ -72,7 +72,7 @@ ErrorOr<FlatPtr> Process::sys$close(int fd)
     TRY(require_promise(Pledge::stdio));
     auto description = TRY(open_file_description(fd));
     auto result = description->close();
-    m_fds.with([fd](auto& fds) { fds[fd] = {}; });
+    m_fds.with_exclusive([fd](auto& fds) { fds[fd] = {}; });
     if (result.is_error())
         return result.release_error();
     return 0;

+ 3 - 3
Kernel/Syscalls/pipe.cpp

@@ -13,7 +13,7 @@ ErrorOr<FlatPtr> Process::sys$pipe(int pipefd[2], int flags)
 {
     VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this)
     TRY(require_promise(Pledge::stdio));
-    auto open_count = fds().with([](auto& fds) { return fds.open_count(); });
+    auto open_count = fds().with_shared([](auto& fds) { return fds.open_count(); });
     if (open_count + 2 > OpenFileDescriptions::max_open())
         return EMFILE;
     // Reject flags other than O_CLOEXEC, O_NONBLOCK
@@ -26,7 +26,7 @@ ErrorOr<FlatPtr> Process::sys$pipe(int pipefd[2], int flags)
     ScopedDescriptionAllocation reader_fd_allocation;
     ScopedDescriptionAllocation writer_fd_allocation;
 
-    TRY(m_fds.with([&](auto& fds) -> ErrorOr<void> {
+    TRY(m_fds.with_exclusive([&](auto& fds) -> ErrorOr<void> {
         reader_fd_allocation = TRY(fds.allocate());
         writer_fd_allocation = TRY(fds.allocate());
         return {};
@@ -42,7 +42,7 @@ ErrorOr<FlatPtr> Process::sys$pipe(int pipefd[2], int flags)
         writer_description->set_blocking(false);
     }
 
-    TRY(m_fds.with([&](auto& fds) -> ErrorOr<void> {
+    TRY(m_fds.with_exclusive([&](auto& fds) -> ErrorOr<void> {
         fds[reader_fd_allocation.fd].set(move(reader_description), fd_flags);
         fds[writer_fd_allocation.fd].set(move(writer_description), fd_flags);
         return {};

+ 1 - 1
Kernel/Syscalls/poll.cpp

@@ -49,7 +49,7 @@ ErrorOr<FlatPtr> Process::sys$poll(Userspace<const Syscall::SC_poll_params*> use
 
     for (size_t i = 0; i < params.nfds; i++) {
         auto& pfd = fds_copy[i];
-        auto description = TRY(m_fds.with([&](auto& fds) { return fds.open_file_description(pfd.fd); }));
+        auto description = TRY(m_fds.with_shared([&](auto& fds) { return fds.open_file_description(pfd.fd); }));
         BlockFlags block_flags = BlockFlags::Exception; // always want POLLERR, POLLHUP, POLLNVAL
         if (pfd.events & POLLIN)
             block_flags |= BlockFlags::Read;

+ 1 - 1
Kernel/Syscalls/read.cpp

@@ -14,7 +14,7 @@ using BlockFlags = Thread::FileBlocker::BlockFlags;
 
 static ErrorOr<NonnullRefPtr<OpenFileDescription>> open_readable_file_description(auto& fds, int fd)
 {
-    auto description = TRY(fds.with([&](auto& fds) { return fds.open_file_description(fd); }));
+    auto description = TRY(fds.with_shared([&](auto& fds) { return fds.open_file_description(fd); }));
     if (!description->is_readable())
         return EBADF;
     if (description->is_directory())

+ 2 - 2
Kernel/Syscalls/sendfd.cpp

@@ -40,7 +40,7 @@ ErrorOr<FlatPtr> Process::sys$recvfd(int sockfd, int options)
     if (!socket.is_local())
         return EAFNOSUPPORT;
 
-    auto fd_allocation = TRY(m_fds.with([](auto& fds) { return fds.allocate(); }));
+    auto fd_allocation = TRY(m_fds.with_exclusive([](auto& fds) { return fds.allocate(); }));
 
     auto& local_socket = static_cast<LocalSocket&>(socket);
     auto received_description = TRY(local_socket.recvfd(*socket_description));
@@ -49,7 +49,7 @@ ErrorOr<FlatPtr> Process::sys$recvfd(int sockfd, int options)
     if (options & O_CLOEXEC)
         fd_flags |= FD_CLOEXEC;
 
-    m_fds.with([&](auto& fds) { fds[fd_allocation.fd].set(move(received_description), fd_flags); });
+    m_fds.with_exclusive([&](auto& fds) { fds[fd_allocation.fd].set(move(received_description), fd_flags); });
     return fd_allocation.fd;
 }
 

+ 6 - 6
Kernel/Syscalls/socket.cpp

@@ -39,7 +39,7 @@ ErrorOr<FlatPtr> Process::sys$socket(int domain, int type, int protocol)
     if ((type & SOCK_TYPE_MASK) == SOCK_RAW && !is_superuser())
         return EACCES;
 
-    return m_fds.with([&](auto& fds) -> ErrorOr<FlatPtr> {
+    return m_fds.with_exclusive([&](auto& fds) -> ErrorOr<FlatPtr> {
         auto fd_allocation = TRY(fds.allocate());
         auto socket = TRY(Socket::create(domain, type, protocol));
         auto description = TRY(OpenFileDescription::try_create(socket));
@@ -51,7 +51,7 @@ ErrorOr<FlatPtr> Process::sys$socket(int domain, int type, int protocol)
 ErrorOr<FlatPtr> Process::sys$bind(int sockfd, Userspace<const sockaddr*> address, socklen_t address_length)
 {
     VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this)
-    return m_fds.with([&](auto& fds) -> ErrorOr<FlatPtr> {
+    return m_fds.with_exclusive([&](auto& fds) -> ErrorOr<FlatPtr> {
         auto description = TRY(fds.open_file_description(sockfd));
         if (!description->is_socket())
             return ENOTSOCK;
@@ -67,7 +67,7 @@ ErrorOr<FlatPtr> Process::sys$listen(int sockfd, int backlog)
     VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this)
     if (backlog < 0)
         return EINVAL;
-    return m_fds.with([&](auto& fds) -> ErrorOr<FlatPtr> {
+    return m_fds.with_exclusive([&](auto& fds) -> ErrorOr<FlatPtr> {
         auto description = TRY(fds.open_file_description(sockfd));
         if (!description->is_socket())
             return ENOTSOCK;
@@ -99,7 +99,7 @@ ErrorOr<FlatPtr> Process::sys$accept4(Userspace<const Syscall::SC_accept4_params
     ScopedDescriptionAllocation fd_allocation;
     RefPtr<OpenFileDescription> accepting_socket_description;
 
-    TRY(m_fds.with([&](auto& fds) -> ErrorOr<void> {
+    TRY(m_fds.with_exclusive([&](auto& fds) -> ErrorOr<void> {
         fd_allocation = TRY(fds.allocate());
         accepting_socket_description = TRY(fds.open_file_description(accepting_socket_fd));
         return {};
@@ -138,7 +138,7 @@ ErrorOr<FlatPtr> Process::sys$accept4(Userspace<const Syscall::SC_accept4_params
     if (flags & SOCK_CLOEXEC)
         fd_flags |= FD_CLOEXEC;
 
-    TRY(m_fds.with([&](auto& fds) -> ErrorOr<void> {
+    TRY(m_fds.with_exclusive([&](auto& fds) -> ErrorOr<void> {
         fds[fd_allocation.fd].set(move(accepted_socket_description), fd_flags);
         return {};
     }));
@@ -361,7 +361,7 @@ ErrorOr<FlatPtr> Process::sys$socketpair(Userspace<const Syscall::SC_socketpair_
 
     auto pair = TRY(LocalSocket::try_create_connected_pair(params.type & SOCK_TYPE_MASK));
 
-    return m_fds.with([&](auto& fds) -> ErrorOr<FlatPtr> {
+    return m_fds.with_exclusive([&](auto& fds) -> ErrorOr<FlatPtr> {
         auto fd_allocation0 = TRY(fds.allocate());
         auto fd_allocation1 = TRY(fds.allocate());