Kernel: Protect Process::m_name with a spinlock

This also lets us remove the `get_process_name` and `set_process_name`
syscalls from the big lock. :^)
This commit is contained in:
Sam Atkins 2023-02-04 13:01:46 +00:00 committed by Andreas Kling
parent b26ecca970
commit fe7b08dad7
Notes: sideshowbarker 2024-07-17 00:44:03 +09:00
13 changed files with 102 additions and 42 deletions

View File

@ -905,7 +905,9 @@ void vdbgln(StringView fmtstr, TypeErasedFormatParams& params)
struct timespec ts = TimeManagement::the().monotonic_time(TimePrecision::Coarse).to_timespec();
if (Kernel::Thread::current()) {
auto& thread = *Kernel::Thread::current();
builder.appendff("{}.{:03} \033[34;1m[#{} {}({}:{})]\033[0m: ", ts.tv_sec, ts.tv_nsec / 1000000, Kernel::Processor::current_id(), thread.process().name(), thread.pid().value(), thread.tid().value());
thread.process().name().with([&](auto& process_name) {
builder.appendff("{}.{:03} \033[34;1m[#{} {}({}:{})]\033[0m: ", ts.tv_sec, ts.tv_nsec / 1000000, Kernel::Processor::current_id(), process_name->view(), thread.pid().value(), thread.tid().value());
});
} else {
builder.appendff("{}.{:03} \033[34;1m[#{} Kernel]\033[0m: ", ts.tv_sec, ts.tv_nsec / 1000000, Kernel::Processor::current_id());
}
@ -958,7 +960,9 @@ void vdmesgln(StringView fmtstr, TypeErasedFormatParams& params)
if (Kernel::Processor::is_initialized() && Kernel::Thread::current()) {
auto& thread = *Kernel::Thread::current();
builder.appendff("{}.{:03} \033[34;1m[{}({}:{})]\033[0m: ", ts.tv_sec, ts.tv_nsec / 1000000, thread.process().name(), thread.pid().value(), thread.tid().value());
thread.process().name().with([&](auto& process_name) {
builder.appendff("{}.{:03} \033[34;1m[{}({}:{})]\033[0m: ", ts.tv_sec, ts.tv_nsec / 1000000, process_name->view(), thread.pid().value(), thread.tid().value());
});
} else {
builder.appendff("{}.{:03} \033[34;1m[Kernel]\033[0m: ", ts.tv_sec, ts.tv_nsec / 1000000);
}
@ -983,7 +987,9 @@ void v_critical_dmesgln(StringView fmtstr, TypeErasedFormatParams& params)
# ifdef AK_OS_SERENITY
if (Kernel::Processor::is_initialized() && Kernel::Thread::current()) {
auto& thread = *Kernel::Thread::current();
builder.appendff("[{}({}:{})]: ", thread.process().name(), thread.pid().value(), thread.tid().value());
thread.process().name().with([&](auto& process_name) {
builder.appendff("[{}({}:{})]: ", process_name->view(), thread.pid().value(), thread.tid().value());
});
} else {
builder.appendff("[Kernel]: ");
}

View File

@ -82,7 +82,7 @@ enum class NeedsBigProcessLock {
S(ftruncate, NeedsBigProcessLock::No) \
S(futex, NeedsBigProcessLock::Yes) \
S(get_dir_entries, NeedsBigProcessLock::Yes) \
S(get_process_name, NeedsBigProcessLock::Yes) \
S(get_process_name, NeedsBigProcessLock::No) \
S(get_root_session_id, NeedsBigProcessLock::No) \
S(get_stack_bounds, NeedsBigProcessLock::No) \
S(get_thread_name, NeedsBigProcessLock::Yes) \
@ -157,7 +157,7 @@ enum class NeedsBigProcessLock {
S(sendmsg, NeedsBigProcessLock::Yes) \
S(set_coredump_metadata, NeedsBigProcessLock::No) \
S(set_mmap_name, NeedsBigProcessLock::Yes) \
S(set_process_name, NeedsBigProcessLock::Yes) \
S(set_process_name, NeedsBigProcessLock::No) \
S(set_thread_name, NeedsBigProcessLock::Yes) \
S(setegid, NeedsBigProcessLock::No) \
S(seteuid, NeedsBigProcessLock::No) \

View File

@ -79,7 +79,7 @@ ErrorOr<void> SysFSOverallProcesses::try_generate(KBufferBuilder& builder)
TRY(process_object.add("tty"sv, ""));
}
TRY(process_object.add("nfds"sv, process.fds().with_shared([](auto& fds) { return fds.open_count(); })));
TRY(process_object.add("name"sv, process.name()));
TRY(process.name().with([&](auto& process_name) { return process_object.add("name"sv, process_name->view()); }));
TRY(process_object.add("executable"sv, process.executable() ? TRY(process.executable()->try_serialize_absolute_path())->view() : ""sv));
size_t amount_virtual = 0;

View File

@ -864,12 +864,14 @@ ErrorOr<CommittedPhysicalPageSet> MemoryManager::commit_physical_pages(size_t pa
amount_shared = space->amount_shared();
amount_virtual = space->amount_virtual();
});
dbgln("{}({}) resident:{}, shared:{}, virtual:{}",
process.name(),
process.pid(),
amount_resident / PAGE_SIZE,
amount_shared / PAGE_SIZE,
amount_virtual / PAGE_SIZE);
process.name().with([&](auto& process_name) {
dbgln("{}({}) resident:{}, shared:{}, virtual:{}",
process_name->view(),
process.pid(),
amount_resident / PAGE_SIZE,
amount_shared / PAGE_SIZE,
amount_virtual / PAGE_SIZE);
});
return IterationDecision::Continue;
});
}

View File

@ -335,10 +335,13 @@ OwnPtr<PerformanceEventBuffer> PerformanceEventBuffer::try_create_with_size(size
ErrorOr<void> PerformanceEventBuffer::add_process(Process const& process, ProcessEventType event_type)
{
OwnPtr<KString> executable;
if (process.executable())
if (process.executable()) {
executable = TRY(process.executable()->try_serialize_absolute_path());
else
executable = TRY(KString::formatted("<{}>", process.name()));
} else {
executable = TRY(process.name().with([&](auto& process_name) {
return KString::formatted("<{}>", process_name->view());
}));
}
TRY(append_with_ip_and_bp(process.pid(), 0, 0, 0,
event_type == ProcessEventType::Create ? PERF_EVENT_PROCESS_CREATE : PERF_EVENT_PROCESS_EXEC,

View File

@ -334,7 +334,11 @@ Process::Process(NonnullOwnPtr<KString> name, NonnullRefPtr<Credentials> credent
protected_data.credentials = move(credentials);
});
dbgln_if(PROCESS_DEBUG, "Created new process {}({})", m_name, this->pid().value());
if constexpr (PROCESS_DEBUG) {
this->name().with([&](auto& process_name) {
dbgln("Created new process {}({})", process_name->view(), this->pid().value());
});
}
}
ErrorOr<void> Process::attach_resources(NonnullOwnPtr<Memory::AddressSpace>&& preallocated_space, LockRefPtr<Thread>& first_thread, Process* fork_parent)
@ -671,7 +675,9 @@ ErrorOr<void> Process::dump_core()
dbgln("Generating coredump for pid {} failed because coredump directory was not set.", pid().value());
return {};
}
auto coredump_path = TRY(KString::formatted("{}/{}_{}_{}", coredump_directory_path->view(), name(), pid().value(), kgettimeofday().to_truncated_seconds()));
auto coredump_path = TRY(name().with([&](auto& process_name) {
return KString::formatted("{}/{}_{}_{}", coredump_directory_path->view(), process_name->view(), pid().value(), kgettimeofday().to_truncated_seconds());
}));
auto coredump = TRY(Coredump::try_create(*this, coredump_path->view()));
return coredump->write();
}
@ -683,7 +689,9 @@ ErrorOr<void> Process::dump_perfcore()
dbgln("Generating perfcore for pid: {}", pid().value());
// Try to generate a filename which isn't already used.
auto base_filename = TRY(KString::formatted("{}_{}", name(), pid().value()));
auto base_filename = TRY(name().with([&](auto& process_name) {
return KString::formatted("{}_{}", process_name->view(), pid().value());
}));
auto perfcore_filename = TRY(KString::formatted("{}.profile", base_filename));
LockRefPtr<OpenFileDescription> description;
auto credentials = this->credentials();
@ -721,8 +729,11 @@ void Process::finalize()
dbgln_if(PROCESS_DEBUG, "Finalizing process {}", *this);
if (veil_state() == VeilState::Dropped)
dbgln("\x1b[01;31mProcess '{}' exited with the veil left open\x1b[0m", name());
if (veil_state() == VeilState::Dropped) {
name().with([&](auto& process_name) {
dbgln("\x1b[01;31mProcess '{}' exited with the veil left open\x1b[0m", process_name->view());
});
}
if (g_init_pid != 0 && pid() == g_init_pid)
PANIC("Init process quit unexpectedly. Exit code: {}", termination_status());
@ -831,11 +842,20 @@ void Process::die()
auto& process = *it;
++it;
if (process.has_tracee_thread(pid())) {
dbgln_if(PROCESS_DEBUG, "Process {} ({}) is attached by {} ({}) which will exit", process.name(), process.pid(), name(), pid());
if constexpr (PROCESS_DEBUG) {
process.name().with([&](auto& process_name) {
name().with([&](auto& name) {
dbgln("Process {} ({}) is attached by {} ({}) which will exit", process_name->view(), process.pid(), name->view(), pid());
});
});
}
process.stop_tracing();
auto err = process.send_signal(SIGSTOP, this);
if (err.is_error())
dbgln("Failed to send the SIGSTOP signal to {} ({})", process.name(), process.pid());
if (err.is_error()) {
process.name().with([&](auto& process_name) {
dbgln("Failed to send the SIGSTOP signal to {} ({})", process_name->view(), process.pid());
});
}
}
}
});
@ -1077,4 +1097,16 @@ ErrorOr<NonnullRefPtr<Custody>> Process::custody_for_dirfd(int dirfd)
return *base_description->custody();
}
SpinlockProtected<NonnullOwnPtr<KString>, LockRank::None> const& Process::name() const
{
return m_name;
}
void Process::set_name(NonnullOwnPtr<KString> name)
{
m_name.with([&](auto& this_name) {
this_name = move(name);
});
}
}

View File

@ -220,7 +220,9 @@ public:
static LockRefPtr<Process> from_pid_ignoring_jails(ProcessID);
static SessionID get_sid_from_pgid(ProcessGroupID pgid);
StringView name() const { return m_name->view(); }
SpinlockProtected<NonnullOwnPtr<KString>, LockRank::None> const& name() const;
void set_name(NonnullOwnPtr<KString>);
ProcessID pid() const
{
return with_protected_data([](auto& protected_data) { return protected_data.pid; });
@ -669,7 +671,7 @@ private:
IntrusiveListNode<Process> m_list_node;
NonnullOwnPtr<KString> m_name;
SpinlockProtected<NonnullOwnPtr<KString>, LockRank::None> m_name;
SpinlockProtected<OwnPtr<Memory::AddressSpace>, LockRank::None> m_space;
@ -1027,7 +1029,9 @@ template<>
struct AK::Formatter<Kernel::Process> : AK::Formatter<FormatString> {
ErrorOr<void> format(FormatBuilder& builder, Kernel::Process const& value)
{
return AK::Formatter<FormatString>::format(builder, "{}({})"sv, value.name(), value.pid().value());
return value.name().with([&](auto& process_name) {
return AK::Formatter<FormatString>::format(builder, "{}({})"sv, process_name->view(), value.pid().value());
});
}
};

View File

@ -647,7 +647,7 @@ ErrorOr<void> Process::do_exec(NonnullLockRefPtr<OpenFileDescription> main_progr
// and we don't want to deal with faults after this point.
auto new_userspace_sp = TRY(make_userspace_context_for_main_thread(new_main_thread->regs(), *load_result.stack_region.unsafe_ptr(), m_arguments, m_environment, move(auxv)));
m_name = move(new_process_name);
set_name(move(new_process_name));
new_main_thread->set_name(move(new_main_thread_name));
if (wait_for_tracer_at_next_execve()) {

View File

@ -27,7 +27,7 @@ ErrorOr<FlatPtr> Process::sys$fork(RegisterState& regs)
}
};
auto child_name = TRY(m_name->try_clone());
auto child_name = TRY(name().with([](auto& name) { return name->try_clone(); }));
auto credentials = this->credentials();
auto child = TRY(Process::try_create(child_first_thread, move(child_name), credentials->uid(), credentials->gid(), pid(), m_is_kernel_process, current_directory(), executable(), m_tty, this));

View File

@ -22,7 +22,9 @@ ErrorOr<void> Process::do_kill(Process& process, int signal)
if (!can_send_signal)
return EPERM;
if (process.is_kernel_process()) {
dbgln("Attempted to send signal {} to kernel process {} ({})", signal, process.name(), process.pid());
process.name().with([&](auto& process_name) {
dbgln("Attempted to send signal {} to kernel process {} ({})", signal, process_name->view(), process.pid());
});
return EPERM;
}
if (signal != 0)

View File

@ -25,18 +25,22 @@ ErrorOr<FlatPtr> Process::sys$getppid()
ErrorOr<FlatPtr> Process::sys$get_process_name(Userspace<char*> buffer, size_t buffer_size)
{
VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this);
VERIFY_NO_PROCESS_BIG_LOCK(this);
TRY(require_promise(Pledge::stdio));
if (m_name->length() + 1 > buffer_size)
return ENAMETOOLONG;
TRY(copy_to_user(buffer, m_name->characters(), m_name->length() + 1));
TRY(m_name.with([&buffer, buffer_size](auto& name) -> ErrorOr<void> {
if (name->length() + 1 > buffer_size)
return ENAMETOOLONG;
return copy_to_user(buffer, name->characters(), name->length() + 1);
}));
return 0;
}
ErrorOr<FlatPtr> Process::sys$set_process_name(Userspace<char const*> user_name, size_t user_name_length)
{
VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this);
VERIFY_NO_PROCESS_BIG_LOCK(this);
TRY(require_promise(Pledge::proc));
if (user_name_length > 256)
return ENAMETOOLONG;
@ -44,7 +48,7 @@ ErrorOr<FlatPtr> Process::sys$set_process_name(Userspace<char const*> user_name,
// Empty and whitespace-only names only exist to confuse users.
if (name->view().is_whitespace())
return EINVAL;
m_name = move(name);
set_name(move(name));
return 0;
}

View File

@ -47,7 +47,9 @@ ErrorOr<FlatPtr> Process::sys$create_thread(void* (*entry)(void*), Userspace<Sys
// We know this thread is not the main_thread,
// So give it a unique name until the user calls $set_thread_name on it
auto new_thread_name = TRY(KString::formatted("{} [{}]", m_name, thread->tid().value()));
auto new_thread_name = TRY(name().with([&](auto& process_name) {
return KString::formatted("{} [{}]", process_name->view(), thread->tid().value());
}));
thread->set_name(move(new_thread_name));
if (!is_thread_joinable)

View File

@ -47,7 +47,7 @@ ErrorOr<NonnullLockRefPtr<Thread>> Thread::try_create(NonnullLockRefPtr<Process>
auto block_timer = TRY(try_make_lock_ref_counted<Timer>());
auto name = TRY(KString::try_create(process->name()));
auto name = TRY(process->name().with([](auto& name) { return name->try_clone(); }));
return adopt_nonnull_lock_ref_or_enomem(new (nothrow) Thread(move(process), move(kernel_stack_region), move(block_timer), move(name)));
}
@ -72,8 +72,11 @@ Thread::Thread(NonnullLockRefPtr<Process> process, NonnullOwnPtr<Memory::Region>
list.append(*this);
});
if constexpr (THREAD_DEBUG)
dbgln("Created new thread {}({}:{})", m_process->name(), m_process->pid().value(), m_tid.value());
if constexpr (THREAD_DEBUG) {
m_process->name().with([&](auto& process_name) {
dbgln("Created new thread {}({}:{})", process_name->view(), m_process->pid().value(), m_tid.value());
});
}
reset_fpu_state();
@ -1467,7 +1470,9 @@ void Thread::track_lock_release(LockRank rank)
ErrorOr<void> AK::Formatter<Kernel::Thread>::format(FormatBuilder& builder, Kernel::Thread const& value)
{
return AK::Formatter<FormatString>::format(
builder,
"{}({}:{})"sv, value.process().name(), value.pid().value(), value.tid().value());
return value.process().name().with([&](auto& process_name) {
return AK::Formatter<FormatString>::format(
builder,
"{}({}:{})"sv, process_name->view(), value.pid().value(), value.tid().value());
});
}