[PATCH v9 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. -- v9: server: Check wineserver privileges on init with -20 niceness. https://gitlab.winehq.org/wine/wine/-/merge_requests/4551
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/thread.c | 44 +++++++++++++++++++++++++++++++------------- server/thread.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/server/thread.c b/server/thread.c index 83ae381d5c5..5c039828912 100644 --- a/server/thread.c +++ b/server/thread.c @@ -606,25 +606,39 @@ 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) + { + errno = EINVAL; + return -1; + } + + if (thread->process->priority == priority_class && + thread->priority == priority) + return 0; + thread->priority = priority; + + return 0; +} + /* 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 ); + if (set_thread_priority( thread, thread->process->priority, req->priority )) + file_set_error(); } if (req->mask & SET_THREAD_INFO_AFFINITY) { @@ -1413,7 +1427,10 @@ DECL_HANDLER(init_first_thread) if (!process->parent_id) process->affinity = current->affinity = get_thread_affinity( current ); else + { + set_thread_priority( current, current->process->priority, current->priority ); set_thread_affinity( current, current->affinity ); + } debug_level = max( debug_level, req->debug_level ); @@ -1444,6 +1461,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 8dcf966a90a..b0237c3a80e 100644 --- a/server/thread.h +++ b/server/thread.h @@ -119,6 +119,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 aa120ae38ec..9dd6c672336 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -2125,6 +2125,9 @@ DECLSPEC_EXPORT void __wine_main( int argc, char *argv[], char *envp[] ) #ifdef RLIMIT_AS set_max_limit( RLIMIT_AS ); #endif +#ifdef RLIMIT_NICE + set_max_limit( RLIMIT_NICE ); +#endif virtual_init(); init_environment( argc, argv, envp ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4551
From: Rémi Bernon <rbernon(a)codeweavers.com> --- configure.ac | 10 ++++++++ server/main.c | 1 + server/object.h | 4 ++++ server/thread.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+) diff --git a/configure.ac b/configure.ac index 074086c586b..b50f1d162aa 100644 --- a/configure.ac +++ b/configure.ac @@ -2090,6 +2090,16 @@ then AC_DEFINE(HAVE_SCHED_SETAFFINITY, 1, [Define to 1 if you have the `sched_setaffinity' function.]) fi +AC_CACHE_CHECK([for setpriority],wine_cv_have_setpriority, + AC_LINK_IFELSE([AC_LANG_PROGRAM( +[[#define _GNU_SOURCE +#include <sys/resource.h> +#include <sys/time.h>]], [[setpriority(0, 0, 0);]])],[wine_cv_have_setpriority=yes],[wine_cv_have_setpriority=no])) +if test "$wine_cv_have_setpriority" = "yes" +then + AC_DEFINE(HAVE_SETPRIORITY, 1, [Define to 1 if you have the `setpriority' function.]) +fi + dnl **** Check for types **** AC_C_INLINE 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 dfdd691601f..66012fbc4af 100644 --- a/server/object.h +++ b/server/object.h @@ -277,6 +277,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 5c039828912..4fe2ad63d9a 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 @@ -215,6 +218,27 @@ static const struct fd_ops thread_fd_ops = }; static struct list thread_list = LIST_INIT(thread_list); +#ifdef __linux__ +static int nice_limit; +#endif + +void init_threading(void) +{ +#ifdef __linux__ +#ifdef RLIMIT_NICE + 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"); + } +#endif + if (nice_limit < 0 && debug_level) fprintf(stderr, "wine: Using setpriority to control niceness in the [%d,%d] range\n", nice_limit, -nice_limit ); +#endif +} /* initialize the structure for a newly allocated thread */ static inline void init_thread_structure( struct thread *thread ) @@ -603,9 +627,46 @@ affinity_t get_thread_affinity( struct thread *thread ) return mask; } +static int get_base_priority( int priority_class, int priority ) +{ + 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 16; + return class_offsets[priority_class - 1] + priority; +} + +#ifdef __linux__ +static int get_unix_niceness( int base_priority ) +{ + int min = -nice_limit, max = nice_limit, range = max - min; + return min + (base_priority - 1) * range / 14; +} +#endif + #define THREAD_PRIORITY_REALTIME_HIGHEST 6 #define THREAD_PRIORITY_REALTIME_LOWEST -7 +static void apply_thread_priority( struct thread *thread, int priority_class, int priority ) +{ + int base_priority = get_base_priority( priority_class, priority ); +#ifdef __linux__ + int niceness; + + /* FIXME: handle REALTIME class using SCHED_RR if possible, for now map it to highest non-realtime band */ + if (priority_class == PROCESS_PRIOCLASS_REALTIME) base_priority = 15; +#ifdef HAVE_SETPRIORITY + if (nice_limit < 0) + { + niceness = get_unix_niceness( base_priority ); + if (setpriority( PRIO_PROCESS, thread->unix_tid, niceness ) != 0) + fprintf( stderr, "wine: setpriority %d for pid %d failed: %d\n", niceness, thread->unix_tid, errno ); + return; + } +#endif +#endif +} + int set_thread_priority( struct thread *thread, int priority_class, int priority ) { int max = THREAD_PRIORITY_HIGHEST; @@ -628,6 +689,7 @@ int set_thread_priority( struct thread *thread, int priority_class, int priority return 0; thread->priority = priority; + apply_thread_priority( thread, priority_class, priority ); return 0; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4551
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/thread.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/server/thread.c b/server/thread.c index 4fe2ad63d9a..86807785062 100644 --- a/server/thread.c +++ b/server/thread.c @@ -227,7 +227,14 @@ void init_threading(void) #ifdef __linux__ #ifdef RLIMIT_NICE struct rlimit rlimit; - if (!getrlimit( RLIMIT_NICE, &rlimit )) +#endif +#ifdef HAVE_SETPRIORITY + /* 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 ); +#endif +#ifdef RLIMIT_NICE + 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
Ah and the last commit message is now also pretty again -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4551#note_54675
This merge request was approved by Marc-Aurel Zent. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4551
Rémi Bernon (@rbernon) commented about server/thread.c:
return mask; }
+static int get_base_priority( int priority_class, int priority ) +{ + 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 16;
Sorry, I forgot where that computation came from, but I found it again (https://learn.microsoft.com/en-us/windows/win32/procthread/scheduling-priori...) and the neutral base priority is 8 (which then also explains the `(base_priority - 1) / 14` below. ```suggestion:-0+0 if (priority_class >= ARRAY_SIZE(class_offsets)) return 8; ``` Could you maybe add a small comment too? Thanks! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4551#note_54733
participants (2)
-
Marc-Aurel Zent (＠mzent) -
Rémi Bernon