[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[microblaze-uclinux] Workaround-Patch for priority inversion in pthread lib of kernel 2.6



Hi!

I experienced hangs of PetaLinux (kernel 2.6) because my multi-threaded user app uses SCHED_RR and runs into thread-inversion problems.
This is a known problem I've already seen in older uClinux releases for the microblaze (with kernel 2.4).

The actual problem is based in the pthread library itself. Once it was discussed here: http://readlist.com/lists/uclinux.org/uclinux-dev/0/3487.html and the final conclusion was that the problem will be fixed with kernel 2.6 together with the NPTL library instead of the pthread library. Unfortunately, pthreads are still used, that's why the problem is still not solved.

My attached patch doesn't fully solve the problem, but fixes the thread-inversion for single-CPU-architectures which I suppose most microblaze systems are. That is why I framed the changes with #ifdef CONFIG_SMP.

Please, can you apply it at least to the PetaLinux distribution?

Cheers,
  Falk

Index: /trunk/software/petalinux-dist/uClibc/libpthread/linuxthreads/semaphore.c
===================================================================
--- /trunk/software/petalinux-dist/uClibc/libpthread/linuxthreads/semaphore.c (revision 2)
+++ /trunk/software/petalinux-dist/uClibc/libpthread/linuxthreads/semaphore.c (revision 173)
@@ -10,20 +10,21 @@
 /* This program is distributed in the hope that it will be useful,      */
 /* but WITHOUT ANY WARRANTY; without even the implied warranty of       */
 /* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the        */
 /* GNU Library General Public License for more details.                 */
 
 /* Semaphores a la POSIX 1003.1b */
 
 #include <features.h>
 #define __USE_GNU
 #include <errno.h>
+#include <asm/system.h>
 #include "pthread.h"
 #include "semaphore.h"
 #include "internals.h"
 #include "spinlock.h"
 #include "restart.h"
 #include "queue.h"
 
 int __new_sem_init(sem_t *sem, int pshared, unsigned int value)
 {
   if (value > SEM_VALUE_MAX) {
@@ -42,54 +43,74 @@
 
 /* Function called by pthread_cancel to remove the thread from
    waiting inside __new_sem_wait. */
 
 static int new_sem_extricate_func(void *obj, pthread_descr th)
 {
   volatile pthread_descr self = thread_self();
   sem_t *sem = obj;
   int did_remove = 0;
 
+#ifdef CONFIG_SMP
   __pthread_lock(&sem->__sem_lock, self);
+#else
+  local_irq_disable();
+#endif
   did_remove = remove_from_queue(&sem->__sem_waiting, th);
+#ifdef CONFIG_SMP
   __pthread_unlock(&sem->__sem_lock);
+#else
+  local_irq_enable();
+#endif
 
   return did_remove;
 }
 
 int __new_sem_wait(sem_t * sem)
 {
   volatile pthread_descr self = thread_self();
   pthread_extricate_if extr;
   int already_canceled = 0;
   int spurious_wakeup_count;
 
   /* Set up extrication interface */
   extr.pu_object = sem;
   extr.pu_extricate_func = new_sem_extricate_func;
 
+#ifdef CONFIG_SMP
   __pthread_lock(&sem->__sem_lock, self);
+#else
+  local_irq_disable();
+#endif
   if (sem->__sem_value > 0) {
     sem->__sem_value--;
+#ifdef CONFIG_SMP
     __pthread_unlock(&sem->__sem_lock);
+#else
+    local_irq_enable();
+#endif
     return 0;
   }
   /* Register extrication interface */
   THREAD_SETMEM(self, p_sem_avail, 0);
   __pthread_set_own_extricate_if(self, &extr);
   /* Enqueue only if not already cancelled. */
   if (!(THREAD_GETMEM(self, p_canceled)
       && THREAD_GETMEM(self, p_cancelstate) == PTHREAD_CANCEL_ENABLE))
     enqueue(&sem->__sem_waiting, self);
   else
     already_canceled = 1;
+#ifdef CONFIG_SMP
   __pthread_unlock(&sem->__sem_lock);
+#else
+  local_irq_enable();
+#endif
 
   if (already_canceled) {
     __pthread_set_own_extricate_if(self, 0);
     pthread_exit(PTHREAD_CANCELED);
   }
 
   /* Wait for sem_post or cancellation, or fall through if already canceled */
   spurious_wakeup_count = 0;
   while (1)
     {
@@ -115,52 +136,76 @@
     pthread_exit(PTHREAD_CANCELED);
   }
   /* We got the semaphore */
   return 0;
 }
 
 int __new_sem_trywait(sem_t * sem)
 {
   int retval;
 
+#ifdef CONFIG_SMP
   __pthread_lock(&sem->__sem_lock, NULL);
+#else
+  local_irq_disable();
+#endif
   if (sem->__sem_value == 0) {
     errno = EAGAIN;
     retval = -1;
   } else {
     sem->__sem_value--;
     retval = 0;
   }
+#ifdef CONFIG_SMP
   __pthread_unlock(&sem->__sem_lock);
+#else
+  local_irq_enable();
+#endif
   return retval;
 }
 
 int __new_sem_post(sem_t * sem)
 {
   pthread_descr self = thread_self();
   pthread_descr th;
   struct pthread_request request;
 
   if (THREAD_GETMEM(self, p_in_sighandler) == NULL) {
+#ifdef CONFIG_SMP
     __pthread_lock(&sem->__sem_lock, self);
+#else
+    local_irq_disable();
+#endif
     if (sem->__sem_waiting == NULL) {
       if (sem->__sem_value >= SEM_VALUE_MAX) {
         /* Overflow */
         errno = ERANGE;
+#ifdef CONFIG_SMP
         __pthread_unlock(&sem->__sem_lock);
+#else
+        local_irq_enable();
+#endif
         return -1;
       }
       sem->__sem_value++;
+#ifdef CONFIG_SMP
       __pthread_unlock(&sem->__sem_lock);
+#else
+      local_irq_enable();
+#endif
     } else {
       th = dequeue(&sem->__sem_waiting);
+#ifdef CONFIG_SMP
       __pthread_unlock(&sem->__sem_lock);
+#else
+      local_irq_enable();
+#endif
       th->p_sem_avail = 1;
       WRITE_MEMORY_BARRIER();
       restart(th);
     }
   } else {
     /* If we're in signal handler, delegate post operation to
        the thread manager. */
     if (__pthread_manager_request < 0) {
       if (__pthread_initialize_manager() < 0) {
         errno = EAGAIN;
@@ -208,66 +253,90 @@
   return -1;
 }
 
 int sem_timedwait(sem_t *sem, const struct timespec *abstime)
 {
   pthread_descr self = thread_self();
   pthread_extricate_if extr;
   int already_canceled = 0;
   int spurious_wakeup_count;
 
+#ifdef CONFIG_SMP
   __pthread_lock(&sem->__sem_lock, self);
+#else
+  local_irq_disable();
+#endif
   if (sem->__sem_value > 0) {
     --sem->__sem_value;
+#ifdef CONFIG_SMP
     __pthread_unlock(&sem->__sem_lock);
+#else
+    local_irq_enable();
+#endif
     return 0;
   }
 
   if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000) {
     /* The standard requires that if the function would block and the
        time value is illegal, the function returns with an error.  */
+#ifdef CONFIG_SMP
     __pthread_unlock(&sem->__sem_lock);
+#else
+    local_irq_enable();
+#endif
     return EINVAL;
   }
 
   /* Set up extrication interface */
   extr.pu_object = sem;
   extr.pu_extricate_func = new_sem_extricate_func;
 
   /* Register extrication interface */
   THREAD_SETMEM(self, p_sem_avail, 0);
   __pthread_set_own_extricate_if(self, &extr);
   /* Enqueue only if not already cancelled. */
   if (!(THREAD_GETMEM(self, p_canceled)
       && THREAD_GETMEM(self, p_cancelstate) == PTHREAD_CANCEL_ENABLE))
     enqueue(&sem->__sem_waiting, self);
   else
     already_canceled = 1;
+#ifdef CONFIG_SMP
   __pthread_unlock(&sem->__sem_lock);
+#else
+  local_irq_enable();
+#endif
 
   if (already_canceled) {
     __pthread_set_own_extricate_if(self, 0);
     pthread_exit(PTHREAD_CANCELED);
   }
 
   spurious_wakeup_count = 0;
   while (1)
     {
       if (timedsuspend(self, abstime) == 0) {
 	int was_on_queue;
 
 	/* __pthread_lock will queue back any spurious restarts that
 	   may happen to it. */
 
+#ifdef CONFIG_SMP
 	__pthread_lock(&sem->__sem_lock, self);
+#else
+	local_irq_disable();
+#endif
 	was_on_queue = remove_from_queue(&sem->__sem_waiting, self);
+#ifdef CONFIG_SMP
 	__pthread_unlock(&sem->__sem_lock);
+#else
+	local_irq_enable();
+#endif
 
 	if (was_on_queue) {
 	  __pthread_set_own_extricate_if(self, 0);
 	  return ETIMEDOUT;
 	}
 
 	/* Eat the outstanding restart() from the signaller */
 	suspend(self);
       }
 
Index: /trunk/software/petalinux-dist/uClibc/libpthread/linuxthreads/spinlock.h
===================================================================
--- /trunk/software/petalinux-dist/uClibc/libpthread/linuxthreads/spinlock.h (revision 2)
+++ /trunk/software/petalinux-dist/uClibc/libpthread/linuxthreads/spinlock.h (revision 173)
@@ -202,17 +202,25 @@
   /* Only store a non-null peif if the thread has cancellation enabled.
      Otherwise pthread_cancel will unconditionally call the extricate handler,
      and restart the thread giving rise to forbidden spurious wakeups. */
   if (peif == NULL
       || THREAD_GETMEM(self, p_cancelstate) == PTHREAD_CANCEL_ENABLE)
     {
       /* If we are removing the extricate interface, we need to synchronize
 	 against pthread_cancel so that it does not continue with a pointer
          to a deallocated pthread_extricate_if struct! The thread lock
          is (ab)used for this synchronization purpose. */
-      if (peif == NULL)
+      if (peif == NULL
+#if !defined(CONFIG_SMP)
+ 		&& THREAD_GETMEM(self, p_cancelstate) == PTHREAD_CANCEL_ENABLE
+#endif
+ 	 )
 	__pthread_lock (THREAD_GETMEM(self, p_lock), self);
       THREAD_SETMEM(self, p_extricate, peif);
-      if (peif == NULL)
+      if (peif == NULL
+#if !defined(CONFIG_SMP)
+            && THREAD_GETMEM(self, p_cancelstate) == PTHREAD_CANCEL_ENABLE
+#endif
+      )
 	__pthread_unlock (THREAD_GETMEM(self, p_lock));
     }
 }