[PATCH v16 0/4] MR4551: Server-side thread priorities implementation (Part 1)
This is the current proton thread priority implementation by @rbernon rebased for upstream with a few `#ifdef`s added since AFAIK Linux is the only operating system where threads have a unique PID which can be used to set niceness on. ~~I also ran `./tools/make_requests` on https://gitlab.winehq.org/mzent/wine/-/commit/6705d3481be0409f7e971c1d2c7a36... as well and `autoconf` on https://gitlab.winehq.org/mzent/wine/-/commit/d7bafe40c411753662b2ad97148a6c... (which does blow up the line count a bit).~~ A few tiny changes ~~(with the ready variable for example)~~ are in anticipation for Part 2, which also adds Mach thread priorities and recalculates thread priorities on process priority change. Since this is a rather large MR, I hope the split here is appropriate ~~(with the second part being slightly smaller)~~, but I think logically it makes the most sense here. -- v16: server: Check wineserver privileges on init with -20 niceness. server: Use setpriority to update thread niceness when safe. ntdll: Set RLIMIT_NICE to its hard limit. server: Introduce new set_thread_priority helper. https://gitlab.winehq.org/wine/wine/-/merge_requests/4551
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/thread.c | 39 ++++++++++++++++++++++++++------------- server/thread.h | 1 + 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/server/thread.c b/server/thread.c index f3880eebedb..21b3b130a9a 100644 --- a/server/thread.c +++ b/server/thread.c @@ -608,25 +608,35 @@ affinity_t get_thread_affinity( struct thread *thread ) #define THREAD_PRIORITY_REALTIME_HIGHEST 6 #define THREAD_PRIORITY_REALTIME_LOWEST -7 +int set_thread_priority( struct thread *thread, int priority_class, int priority ) +{ + int max = THREAD_PRIORITY_HIGHEST; + int min = THREAD_PRIORITY_LOWEST; + if (priority_class == PROCESS_PRIOCLASS_REALTIME) + { + max = THREAD_PRIORITY_REALTIME_HIGHEST; + min = THREAD_PRIORITY_REALTIME_LOWEST; + } + if ((priority < min || priority > max) && + priority != THREAD_PRIORITY_IDLE && + priority != THREAD_PRIORITY_TIME_CRITICAL) + return STATUS_INVALID_PARAMETER; + + if (thread->state == TERMINATED) + return STATUS_THREAD_IS_TERMINATING; + + thread->priority = priority; + return STATUS_SUCCESS; +} + /* set all information about a thread */ static void set_thread_info( struct thread *thread, const struct set_thread_info_request *req ) { if (req->mask & SET_THREAD_INFO_PRIORITY) { - int max = THREAD_PRIORITY_HIGHEST; - int min = THREAD_PRIORITY_LOWEST; - if (thread->process->priority == PROCESS_PRIOCLASS_REALTIME) - { - max = THREAD_PRIORITY_REALTIME_HIGHEST; - min = THREAD_PRIORITY_REALTIME_LOWEST; - } - if ((req->priority >= min && req->priority <= max) || - req->priority == THREAD_PRIORITY_IDLE || - req->priority == THREAD_PRIORITY_TIME_CRITICAL) - thread->priority = req->priority; - else - set_error( STATUS_INVALID_PARAMETER ); + int status = set_thread_priority( thread, thread->process->priority, req->priority ); + if (status) set_error( status ); } if (req->mask & SET_THREAD_INFO_AFFINITY) { @@ -1422,6 +1432,8 @@ DECL_HANDLER(init_first_thread) else set_thread_affinity( current, current->affinity ); + set_thread_priority( current, process->priority, current->priority ); + debug_level = max( debug_level, req->debug_level ); reply->pid = get_process_id( process ); @@ -1451,6 +1463,7 @@ DECL_HANDLER(init_thread) init_thread_context( current ); generate_debug_event( current, DbgCreateThreadStateChange, &req->entry ); + set_thread_priority( current, current->process->priority, current->priority ); set_thread_affinity( current, current->affinity ); reply->suspend = (current->suspend || current->process->suspend || current->context != NULL); diff --git a/server/thread.h b/server/thread.h index 3448f332b0b..dd5dc864373 100644 --- a/server/thread.h +++ b/server/thread.h @@ -122,6 +122,7 @@ extern void thread_cancel_apc( struct thread *thread, struct object *owner, enum extern int thread_add_inflight_fd( struct thread *thread, int client, int server ); extern int thread_get_inflight_fd( struct thread *thread, int client ); extern struct token *thread_get_impersonation_token( struct thread *thread ); +extern int set_thread_priority( struct thread *thread, int priority_class, int priority ); extern int set_thread_affinity( struct thread *thread, affinity_t affinity ); extern int suspend_thread( struct thread *thread ); extern int resume_thread( struct thread *thread ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4551
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/unix/loader.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index 92f2e2eb3a3..396e1e624f2 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -2178,6 +2178,9 @@ DECLSPEC_EXPORT void __wine_main( int argc, char *argv[] ) #ifdef RLIMIT_AS set_max_limit( RLIMIT_AS ); #endif +#ifdef RLIMIT_NICE + set_max_limit( RLIMIT_NICE ); +#endif virtual_init(); init_environment(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4551
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- server/main.c | 1 + server/object.h | 4 ++++ server/thread.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/server/main.c b/server/main.c index 1248b92f24d..e014ec535ff 100644 --- a/server/main.c +++ b/server/main.c @@ -234,6 +234,7 @@ int main( int argc, char *argv[] ) init_signals(); init_memory(); init_directories( load_intl_file() ); + init_threading(); init_registry(); main_loop(); return 0; diff --git a/server/object.h b/server/object.h index 6222e3352ed..2268c997437 100644 --- a/server/object.h +++ b/server/object.h @@ -287,6 +287,10 @@ extern struct object *get_directory_obj( struct process *process, obj_handle_t h extern int directory_link_name( struct object *obj, struct object_name *name, struct object *parent ); extern void init_directories( struct fd *intl_fd ); +/* thread functions */ + +extern void init_threading(void); + /* symbolic link functions */ extern struct object *create_root_symlink( struct object *root, const struct unicode_str *name, diff --git a/server/thread.c b/server/thread.c index 21b3b130a9a..c07a2338c50 100644 --- a/server/thread.c +++ b/server/thread.c @@ -37,6 +37,9 @@ #define _WITH_CPU_SET_T #include <sched.h> #endif +#ifdef HAVE_SYS_RESOURCE_H +#include <sys/resource.h> +#endif #include "ntstatus.h" #define WIN32_NO_STATUS @@ -216,6 +219,45 @@ static const struct fd_ops thread_fd_ops = static struct list thread_list = LIST_INIT(thread_list); +#if defined(__linux__) && defined(RLIMIT_NICE) +static int nice_limit; + +void init_threading(void) +{ + struct rlimit rlimit; + if (!getrlimit( RLIMIT_NICE, &rlimit )) + { + rlimit.rlim_cur = rlimit.rlim_max; + setrlimit( RLIMIT_NICE, &rlimit ); + if (rlimit.rlim_max <= 40) nice_limit = 20 - rlimit.rlim_max; + else if (rlimit.rlim_max == -1) nice_limit = -20; + if (nice_limit >= 0 && debug_level) fprintf(stderr, "wine: RLIMIT_NICE is <= 20, unable to use setpriority safely\n"); + } + if (nice_limit < 0 && debug_level) fprintf(stderr, "wine: Using setpriority to control niceness in the [%d,%d] range\n", nice_limit, -nice_limit ); +} + +static void apply_thread_priority( struct thread *thread, int base_priority ) +{ + int min = -nice_limit, max = nice_limit, range = max - min, niceness; + /* FIXME: handle realtime priorities using SCHED_RR if possible */ + if (base_priority > THREAD_BASE_PRIORITY_LOWRT) base_priority = THREAD_BASE_PRIORITY_LOWRT; + /* map an NT application band [1,15] base priority to [-nice_limit, nice_limit] */ + niceness = (min + (base_priority - 1) * range / 14); + setpriority( PRIO_PROCESS, thread->unix_tid, niceness ); +} + +#else + +void init_threading(void) +{ +} + +static void apply_thread_priority( struct thread *thread, int base_priority ) +{ +} + +#endif + /* initialize the structure for a newly allocated thread */ static inline void init_thread_structure( struct thread *thread ) { @@ -605,13 +647,23 @@ affinity_t get_thread_affinity( struct thread *thread ) return mask; } +static int get_base_priority( int priority_class, int priority ) +{ + /* offsets taken from https://learn.microsoft.com/en-us/windows/win32/procthread/scheduling-priori... */ + static const int class_offsets[] = { 4, 8, 13, 24, 6, 10 }; + if (priority == THREAD_PRIORITY_IDLE) return (priority_class == PROCESS_PRIOCLASS_REALTIME ? 16 : 1); + if (priority == THREAD_PRIORITY_TIME_CRITICAL) return (priority_class == PROCESS_PRIOCLASS_REALTIME ? 31 : 15); + if (priority_class >= ARRAY_SIZE(class_offsets)) return 8; + return class_offsets[priority_class - 1] + priority; +} + #define THREAD_PRIORITY_REALTIME_HIGHEST 6 #define THREAD_PRIORITY_REALTIME_LOWEST -7 int set_thread_priority( struct thread *thread, int priority_class, int priority ) { - int max = THREAD_PRIORITY_HIGHEST; - int min = THREAD_PRIORITY_LOWEST; + int min = THREAD_PRIORITY_LOWEST, max = THREAD_PRIORITY_HIGHEST, base_priority; + if (priority_class == PROCESS_PRIOCLASS_REALTIME) { max = THREAD_PRIORITY_REALTIME_HIGHEST; @@ -626,6 +678,13 @@ int set_thread_priority( struct thread *thread, int priority_class, int priority return STATUS_THREAD_IS_TERMINATING; thread->priority = priority; + + /* if unix_tid == -1, thread is gone or hasn't started yet, this will be called again from init_thread with a unix_tid */ + if (thread->unix_tid == -1) + return STATUS_SUCCESS; + + base_priority = get_base_priority( priority_class, priority ); + apply_thread_priority( thread, base_priority ); return STATUS_SUCCESS; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4551
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/thread.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server/thread.c b/server/thread.c index c07a2338c50..b742e0e693d 100644 --- a/server/thread.c +++ b/server/thread.c @@ -225,7 +225,12 @@ static int nice_limit; void init_threading(void) { struct rlimit rlimit; - if (!getrlimit( RLIMIT_NICE, &rlimit )) + + /* if wineserver has cap_sys_nice we are unlimited, but leave -20 to the user */ + if (!setpriority( PRIO_PROCESS, getpid(), -20 )) nice_limit = -19; + setpriority( PRIO_PROCESS, getpid(), 0 ); + + if (!nice_limit && !getrlimit( RLIMIT_NICE, &rlimit )) { rlimit.rlim_cur = rlimit.rlim_max; setrlimit( RLIMIT_NICE, &rlimit ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4551
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=149399 Your paranoid android. === debian11 (build log) === error: patch failed: server/thread.c:608 error: patch failed: server/thread.h:122 error: patch failed: dlls/ntdll/unix/loader.c:2178 error: patch failed: server/thread.c:605 error: patch failed: server/thread.c:225 Task: Patch failed to apply === debian11b (build log) === error: patch failed: server/thread.c:608 error: patch failed: server/thread.h:122 error: patch failed: dlls/ntdll/unix/loader.c:2178 error: patch failed: server/thread.c:605 error: patch failed: server/thread.c:225 Task: Patch failed to apply
I'm not sure what is expected here, but after having another look I think it still looks good. Also fwiw I've updated Proton with the latest revision of the MR a couple of weeks ago and it still works as intended as far as I can tell. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4551#note_87609
participants (4)
-
Marc-Aurel Zent -
Marc-Aurel Zent (＠mzent)
-
Marvin -
Rémi Bernon