Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linux: fix thread priority #14252

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
+20 −0
Draft

Linux: fix thread priority #14252

Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
8163aaf
Linux: fix thread priority
hoholee12 Jul 23, 2023
e5c1fea
oops
hoholee12 Jul 23, 2023
cf785e2
oops
hoholee12 Jul 23, 2023
3d1f527
FIx Cirrus Linux build not compiling
hoholee12 Jul 24, 2023
45023d2
Merge branch 'master' into master
elad335 Jul 24, 2023
db4c960
Root user gets proper privilege
hoholee12 Jul 25, 2023
445bae1
Root user gets proper privilege
hoholee12 Jul 25, 2023
b9257cf
remove spaces
hoholee12 Jul 25, 2023
3795753
Merge branch 'master' into master
elad335 Jul 25, 2023
4300857
remove nonroot
hoholee12 Jul 25, 2023
ca381b0
Merge branch 'master' of https://github.com/hoholee12/rpcs3
hoholee12 Jul 25, 2023
0ce786c
fix
hoholee12 Jul 25, 2023
328d73c
Merge branch 'master' into master
hoholee12 Jul 28, 2023
4695e9c
Merge branch 'master' into master
hoholee12 Sep 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions Utilities/Thread.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ DYNAMIC_IMPORT_RENAME("Kernel32.dll", SetThreadDescriptionImport, "SetThreadDesc
#ifdef __linux__
#include <sys/timerfd.h>
#include <unistd.h>
#include <sys/syscall.h>
#define gettid() syscall(SYS_gettid)
#endif

#if defined(__APPLE__) || defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__)
Expand Down Expand Up @@ -3028,6 +3030,39 @@ void thread_ctrl::set_native_priority(int priority)
{
sig_log.error("SetThreadPriority() failed: %s", fmt::win_error{GetLastError(), nullptr});
}
#elif defined(__linux__)
// available niceness for nonroot: 0~19
// available niceness for root: -20~19

int linuxprio = 0;
Megamouse marked this conversation as resolved.
Show resolved Hide resolved
hoholee12 marked this conversation as resolved.
Show resolved Hide resolved
id_t threadpid = gettid();
uid_t euid = geteuid();

if (euid == 0)
{
linuxprio = 0;
if (priority > 0)
linuxprio = -6;
else if (priority < 0)
linuxprio = 6;
}
else
{
linuxprio = 6;
if (priority > 0)
linuxprio = 0;
else if (priority < 0)
linuxprio = 12;
}

// nonroot cannot increase niceness value
if ((getpriority(PRIO_PROCESS, threadpid) < linuxprio) || (euid == 0))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, if a thread sets lower priority, it cannot increase it afterwards if it is non-root?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately, yep

Copy link
Contributor

@elad335 elad335 Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a problem because, named_thread is recycled from a pool after use. So it would get stuck with lowered priority.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. i noticed this by testing alot and game threads are stuck with low priority. im thinking maybe we only should allow priority changes for root user and disregard nonroots...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so too.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this handle the case where a user might have the appropriate rights (limits.conf or CAP_SYS_NICE)?

{
if (int err = setpriority(PRIO_PROCESS, threadpid, linuxprio))
{
sig_log.error("setpriority(%d, %d) failed: %d", threadpid, linuxprio, err);
}
}
#else
int policy;
struct sched_param param;
Expand Down