Re: setpriority() again

[Sebastian Wilhelmi <seppi seppi de>]

Hi Tim,

> I'm currently coding a suid wrapper for beast to increase
> nicety of the sound threads. basically, things work as
> follows:
> 1) suidwrapper starts, does setpriority (PRIO_PROCESS, getpid(), PRIO_MIN); (i.e. nice=-20)
> 2) drops suid-ness to euid
> 3) suidwrapper executes beast
> 4) beast starts BSE thread
> 5) beast drops priority (via setpriority (PRIO_PROCESS, getpid(), 0); since
>    i have no idea how to use g_thread_set_priority() to get exactly nice=0)
> 6) BSE thread starts SoundSequencer thread (with G_THREAD_PRIORITY_NORMAL)
> 7) upon user action: BSE thread starts SoundEngine thread (with G_THREAD_PRIORITY_NORMAL)
> 
> now, upon execution of (7) i'm getting:
> 
> (BEAST:21727): GLib-WARNING **: Priorities can only be increased by root.
> 
> the system setup i'm using is linux-2.6.1, glibc-2.3.2, gcc-3.3, glib-2.2.3.
> 
> the code for PID priorities in gthread.c does:
>   priority_map[G_THREAD_PRIORITY_NORMAL] = getpriority (PRIO_PROCESS, (getpid ()));
>   priority_map[G_THREAD_PRIORITY_LOW] = MIN (20, priority_map[G_THREAD_PRIORITY_NORMAL] + 10);
>   priority_map[G_THREAD_PRIORITY_HIGH] = MAX (-20, priority_map[G_THREAD_PRIORITY_NORMAL] - 10);
>   priority_map[G_THREAD_PRIORITY_URGENT] = MAX (-20, priority_map[G_THREAD_PRIORITY_NORMAL] - 15);
> and i'm creating my threads with G_THREAD_PRIORITY_NORMAL.
> 
> note that under NPTL, getpid() returns the same value for all the threads
> created under 4, 5, 6 and 7. so basically what happens is:
> - when beast starts and does g_thread_init(),
>   priority_map[G_THREAD_PRIORITY_NORMAL] is set to -20.
> - the BSE thread and SoundSequencer threads are started *before* (5) is
>   being executed.
> - the newly started threads do
>   setpriority(getpid(),priority_map[G_THREAD_PRIORITY_NORMAL]) and succeed.
>   i.e. they try to set the priority of *beast* to a priority they and beast
>   already have,
> - beast drops priority to 0, i.e. (5) is being executed
> - the SoundEngine thread started tries to
>   setpriority(getpid(),priority_map[G_THREAD_PRIORITY_NORMAL]), i.e. tries
>   to set its own priority to a priority it already has (namely -20), but
>   fails to set the *beast* priority (now 0) to -20.
> 
> i think we should really disable the PID surrogate stuff, there's not much
> point in renicing the same pid over and over with newer linuxes. and even
> with older ones where this approach still works, it's pretty pointless to
> reset priorities upon thread creation to a priority the main thread once had.

Actually that wont happen if you configure on that machine. But using a
package compiled on an older kernel/glibc combination will still show
this odd behavior. This PID code indeed is a very crude hack and I would
be very happy, if we could get rid of it. I implemented it to get thread
priorities on linux, arguably our main platform. If we don't have thread
priorities on linux, we could as well drop thread priority support from
GLib alltogether.

But we could as well fix the PID thing. We could call gettid to get the
thread id and use it to renice the thread. This works well. I have
attached a patch to do that and I will check it in, if no-one objects.
It also works on 2.4 Kernels. It might not work on 2.2. kernels, but
that is caught by the configure.in check.

> we should face it: thread priorities as provided by posix are a different
> beast from pid niceties and thus need to be handled differently. in particular,
> pid nicety handling has to be left up to the user, to not hinder:
> - nice-level settings via a config dialog/file or command line option (here,
>   users will want to specify exact nice levels)
> - nice-level preparation via sudo wrappers, glib shouldn't interfere with that
>   process by forcing nice-level resets
> - ordinary idle cruncher nicing.

I agree, that setting niceness values is bad, if no G_THREAD_PRIORITY_*
is used. I'll fix it.

> especially for the third scenario, it's more than a nuisance that
> g_thread_set_priority() throws a programming warning if it fails for whatever
> reason. e.g. trying to nice an idle cruncher to +5 via g_thread_set_priority()
> throws a programming warning if the user started the program with nice levels >=6.
> in a different scenario, the idle cruncher might renice itself by means of invoking
> nice(+5) directly (since g_thread_set_priority() can't be used to accurately set
> nice levels). in this case, it can't start anymore threads, since every new thread
> will try to either renice itself or the main thread (NPTL) to the better original
> nice level (-5 from the current, only allowed by root).

Note however that the warning is printed once only.

> side note: stefan, due to getpid() returning the same pid for all threads,
>    i'm not able to slightly lower the priority of the BSE thread compared to
>    the SoundSequencer/SoundEngine threads.

man gettid.

Ciao,
Sebastian
-- 
Sebastian Wilhelmi                 |            här ovanför alla molnen
mailto:seppi seppi de              |     är himmlen så förunderligt blå
http://seppi.de                    |

Index: configure.in
===================================================================
RCS file: /cvs/gnome/glib/configure.in,v
retrieving revision 1.381
diff -u -r1.381 configure.in
--- configure.in	24 Feb 2004 19:27:09 -0000	1.381
+++ configure.in	25 Feb 2004 13:16:42 -0000
@@ -1767,16 +1767,19 @@
                  [AC_TRY_RUN([
 #include <pthread.h> 
 #include <sys/types.h>
-#include <unistd.h>
+#include <linux/unistd.h>
+#include <unistd.h> 
+_syscall0(pid_t,gettid)
+
 pid_t other_pid = 0;
 
-void* func(void* data) {other_pid = getpid();}
+void* func(void* data) {other_pid = gettid();}
 main()
 { pthread_t t; 
   void *ret;
   pthread_create (&t, 0, func, NULL);
   pthread_join (t, &ret);
-  exit (getpid()==other_pid || 
+  exit (gettid()==other_pid || 
 	$posix_priority_min != $posix_priority_max);
 }               ],
 		[glib_cv_use_pid_surrogate=yes],
Index: glib/gthread.c
===================================================================
RCS file: /cvs/gnome/glib/glib/gthread.c,v
retrieving revision 1.35
diff -u -r1.35 gthread.c
--- glib/gthread.c	8 Jul 2003 23:43:37 -0000	1.35
+++ glib/gthread.c	25 Feb 2004 13:16:47 -0000
@@ -39,6 +39,8 @@
 #include <sys/time.h>
 #include <sys/resource.h>
 #include <errno.h>
+#include <linux/unistd.h>
+_syscall0(pid_t,gettid)
 #endif /* G_THREAD_USE_PID_SURROGATE */
 
 #ifdef HAVE_UNISTD_H
@@ -85,15 +87,15 @@
   gpointer retval;
   GSystemThread system_thread;
 #ifdef G_THREAD_USE_PID_SURROGATE
-  pid_t pid;
+  pid_t tid;
 #endif /* G_THREAD_USE_PID_SURROGATE */
 };
 
 #ifdef G_THREAD_USE_PID_SURROGATE
 static gint priority_map[4];
 static gboolean prio_warned = FALSE;
-# define SET_PRIO(pid, prio) G_STMT_START{				\
-  gint error = setpriority (PRIO_PROCESS, (pid), priority_map[prio]);	\
+# define SET_PRIO(tid, prio) G_STMT_START{				\
+  gint error = setpriority (PRIO_PROCESS, (tid), priority_map[prio]);	\
   if (error == -1 && errno == EACCES && !prio_warned)			\
     {									\
       prio_warned = TRUE;						\
@@ -179,7 +181,7 @@
   
 #ifdef G_THREAD_USE_PID_SURROGATE
   priority_map[G_THREAD_PRIORITY_NORMAL] = 
-    getpriority (PRIO_PROCESS, (getpid ()));
+    getpriority (PRIO_PROCESS, (gettid ()));
   priority_map[G_THREAD_PRIORITY_LOW] = 
     MIN (20, priority_map[G_THREAD_PRIORITY_NORMAL] + 10);
   priority_map[G_THREAD_PRIORITY_HIGH] = 
@@ -582,7 +584,7 @@
   g_assert (data);
 
 #ifdef G_THREAD_USE_PID_SURROGATE
-  thread->pid = getpid ();
+  thread->tid = gettid ();
 #endif /* G_THREAD_USE_PID_SURROGATE */
 
   /* This has to happen before G_LOCK, as that might call g_thread_self */
@@ -595,7 +597,7 @@
  
 #ifdef G_THREAD_USE_PID_SURROGATE
   if (g_thread_use_default_impl)
-    SET_PRIO (thread->pid, thread->thread.priority);
+    SET_PRIO (thread->tid, thread->thread.priority);
 #endif /* G_THREAD_USE_PID_SURROGATE */
 
   thread->retval = thread->thread.func (thread->thread.data);
@@ -697,7 +699,7 @@
 
 #ifdef G_THREAD_USE_PID_SURROGATE
   if (g_thread_use_default_impl)
-    SET_PRIO (real->pid, priority);
+    SET_PRIO (real->tid, priority);
   else
 #endif /* G_THREAD_USE_PID_SURROGATE */
     G_THREAD_CF (thread_set_priority, (void)0, 
@@ -726,7 +728,7 @@
 	G_THREAD_UF (thread_self, (&thread->system_thread));
 
 #ifdef G_THREAD_USE_PID_SURROGATE
-      thread->pid = getpid ();
+      thread->tid = gettid ();
 #endif /* G_THREAD_USE_PID_SURROGATE */
       
       g_private_set (g_thread_specific_private, thread);