From 2eb17701ef4fe8e270f5056acf108fea244c8ee0 Mon Sep 17 00:00:00 2001
From: sasan-golchin <ahmad.golchin@mail.polimi.it>
Date: Mon, 7 Nov 2016 13:50:44 +0100
Subject: [PATCH] Threads' priority for preemption decoupled from the one for
 priority inheritance(the sync module)

---
 miosix/kernel/kernel.cpp                      | 19 ++++----
 miosix/kernel/kernel.h                        |  2 +-
 miosix/kernel/pthread_private.h               |  2 +-
 miosix/kernel/queue.h                         |  8 ++--
 .../scheduler/control/control_scheduler.cpp   | 19 ++++----
 .../scheduler/control/control_scheduler.h     |  2 +
 .../control/control_scheduler_types.h         | 43 ++++++++++---------
 .../scheduler/priority/priority_scheduler.cpp | 20 ++++++---
 .../scheduler/priority/priority_scheduler.h   |  2 +
 .../priority/priority_scheduler_types.h       | 22 +++++-----
 miosix/kernel/scheduler/scheduler.h           | 12 +++++-
 miosix/kernel/scheduler/timer_interrupt.h     |  9 ++--
 miosix/kernel/sync.cpp                        | 24 ++++++-----
 13 files changed, 111 insertions(+), 73 deletions(-)

diff --git a/miosix/kernel/kernel.cpp b/miosix/kernel/kernel.cpp
index f6372247..202b1de2 100755
--- a/miosix/kernel/kernel.cpp
+++ b/miosix/kernel/kernel.cpp
@@ -272,7 +272,8 @@ void IRQaddToSleepingList(SleepData *x)
         while (it != sleepingList->end() && (*it)->wakeup_time < x->wakeup_time ) ++it;
         sleepingList->insert(it,x);
     }
-    //firstSleepItemTicks = tc->ns2tick(sleepingList->front()->wakeup_time);
+    //if (sleepingList->front()->wakeup_time < ContextSwitchTimer::instance().IRQgetCurrentTime())
+    //    ContextSwitchTimer::instance().IRQsetNextInterrupt(sleepingList->front()->wakeup_time);
 }
 
 /**
@@ -281,7 +282,7 @@ void IRQaddToSleepingList(SleepData *x)
  * Also increases the system tick.
  * Takes care of clearing SLEEP_FLAG.
  * It is used by the kernel, and should not be used by end users.
- * \return true if some thread was woken.
+ * \return true if some thread with higher priority of current thread is woken.
  */
 bool IRQwakeThreads(long long currentTick)
 {
@@ -298,14 +299,12 @@ bool IRQwakeThreads(long long currentTick)
         if((*it)->p == nullptr) ++it; //Only csRecord has p==nullptr
         else {
             (*it)->p->flags.IRQsetSleep(false); //Wake thread
+            if (const_cast<Thread*>(cur)->getPriority() < (*it)->p->getPriority())
+                result = true;
             it = sleepingList->erase(it);
-            result = true;
+            
         }
     }
-//    if(sleepingList->empty()) 
-//        firstSleepItemTicks = std::numeric_limits<long long>::max();
-//    else
-//        firstSleepItemTicks = tc->ns2tick(sleepingList->front()->wakeup_time);
     return result;
 }
 
@@ -394,8 +393,12 @@ void Thread::nanoSleepUntil(long long absoluteTime)
         d.wakeup_time = absoluteTime;
         IRQaddToSleepingList(&d);//Also sets SLEEP_FLAG
     }
+    // NOTE: There is no need to synchronize the timer (calling IRQsetNextInterrupt)
+    // with the list at this point. Because, Thread::yield will make a supervisor
+    // call and subsequently it will call the IRQfindNextThread. IRQfindNextThread
+    // keeps the timer synchronized with the sleeping list head and beginning of
+    // next burst time. (see ISR_yield() in portability.cpp
     Thread::yield();
-    
 }
 
 void Thread::sleep(unsigned int ms)
diff --git a/miosix/kernel/kernel.h b/miosix/kernel/kernel.h
index d675a650..e2ae35d7 100755
--- a/miosix/kernel/kernel.h
+++ b/miosix/kernel/kernel.h
@@ -1068,7 +1068,7 @@ public:
      */
     bool operator() (Thread* a, Thread *b)
     {
-        return a->getPriority() < b->getPriority();
+        return a->getPriority().mutexLessOp(b->getPriority());
     }
 };
 
diff --git a/miosix/kernel/pthread_private.h b/miosix/kernel/pthread_private.h
index 54c47156..b05288c6 100644
--- a/miosix/kernel/pthread_private.h
+++ b/miosix/kernel/pthread_private.h
@@ -172,7 +172,7 @@ static inline bool IRQdoMutexUnlock(pthread_mutex_t *mutex)
         mutex->first=mutex->first->next;
 
         #ifndef SCHED_TYPE_EDF
-        if(t->IRQgetPriority() >Thread::IRQgetCurrentThread()->IRQgetPriority())
+        if(Thread::IRQgetCurrentThread()->IRQgetPriority() < t->IRQgetPriority())
             return true;
         #endif //SCHED_TYPE_EDF
         return false;
diff --git a/miosix/kernel/queue.h b/miosix/kernel/queue.h
index 34dcf5fe..eb8d78e4 100644
--- a/miosix/kernel/queue.h
+++ b/miosix/kernel/queue.h
@@ -277,8 +277,8 @@ bool Queue<T,len>::IRQget(T& elem)
 template <typename T, unsigned int len>
 bool Queue<T,len>::IRQget(T& elem, bool& hppw)
 {
-    if(waiting && (waiting->IRQgetPriority() >
-            Thread::IRQgetCurrentThread()->IRQgetPriority())) hppw=true;
+    if(waiting && (Thread::IRQgetCurrentThread()->IRQgetPriority()) < 
+            waiting->IRQgetPriority()) hppw=true;
     IRQwakeWaitingThread();
     if(isEmpty()) return false;
     numElem--;
@@ -301,8 +301,8 @@ bool Queue<T,len>::IRQput(const T& elem)
 template <typename T, unsigned int len>
 bool Queue<T,len>::IRQput(const T& elem, bool& hppw)
 {
-    if(waiting && (waiting->IRQgetPriority() >
-            Thread::IRQgetCurrentThread()->IRQgetPriority())) hppw=true;
+    if(waiting && (Thread::IRQgetCurrentThread()->IRQgetPriority() <
+            waiting->IRQgetPriority())) hppw=true;
     IRQwakeWaitingThread();
     if(isFull()) return false;
     numElem++;
diff --git a/miosix/kernel/scheduler/control/control_scheduler.cpp b/miosix/kernel/scheduler/control/control_scheduler.cpp
index 3c78b955..3de8f97b 100644
--- a/miosix/kernel/scheduler/control/control_scheduler.cpp
+++ b/miosix/kernel/scheduler/control/control_scheduler.cpp
@@ -49,6 +49,7 @@ static ContextSwitchTimer& timer = ContextSwitchTimer::instance();
 static long long burstStart = 0;
 static IntrusiveList<ThreadsListItem> activeThreads;
 static IntrusiveList<ThreadsListItem>::iterator curInRound = activeThreads.end();
+static long long nextPreemption = numeric_limits<long long>::max();
 
 //
 // class ControlScheduler
@@ -191,28 +192,30 @@ Thread *ControlScheduler::IRQgetIdleThread()
     return idle;
 }
 
+long long ControlScheduler::IRQgetNextPreemption()
+{
+    return nextPreemption;
+}
+
 // Should be called when the current thread is the idle thread
 static inline void IRQsetNextPreemptionForIdle(){
     if (sleepingList->empty())
-        timer.IRQsetNextInterrupt(numeric_limits<long long>::max());
+        nextPreemption = numeric_limits<long long>::max();
     else
-        timer.IRQsetNextInterrupt(sleepingList->front()->wakeup_time);
+        nextPreemption = sleepingList->front()->wakeup_time;
+    timer.IRQsetNextInterrupt(nextPreemption);
 }
 
 // Should be called for threads other than idle thread
 static inline void IRQsetNextPreemption(long long burst){
     long long firstWakeupInList;
-    long long nextPreemption;
     if (sleepingList->empty())
         firstWakeupInList = numeric_limits<long long>::max();
     else
         firstWakeupInList = sleepingList->front()->wakeup_time;
     burstStart = timer.IRQgetCurrentTime();
-    nextPreemption = burstStart + burst;
-    if (firstWakeupInList < nextPreemption)
-        timer.IRQsetNextInterrupt(firstWakeupInList);
-    else
-        timer.IRQsetNextInterrupt(nextPreemption);
+    nextPreemption = min(firstWakeupInList,burstStart + burst);
+    timer.IRQsetNextInterrupt(nextPreemption);
 }
 
 unsigned int ControlScheduler::IRQfindNextThread()
diff --git a/miosix/kernel/scheduler/control/control_scheduler.h b/miosix/kernel/scheduler/control/control_scheduler.h
index 79221bd6..08660520 100755
--- a/miosix/kernel/scheduler/control/control_scheduler.h
+++ b/miosix/kernel/scheduler/control/control_scheduler.h
@@ -146,6 +146,8 @@ public:
      * points to the currently running thread.
      */
     static unsigned int IRQfindNextThread();
+    
+    static long long IRQgetNextPreemption();
 
 private:
     
diff --git a/miosix/kernel/scheduler/control/control_scheduler_types.h b/miosix/kernel/scheduler/control/control_scheduler_types.h
index c1cc1284..f6860878 100755
--- a/miosix/kernel/scheduler/control/control_scheduler_types.h
+++ b/miosix/kernel/scheduler/control/control_scheduler_types.h
@@ -57,11 +57,11 @@ class Thread; //Forward declaration
  */
 #define REALTIME_PRIORITY_NEXT_BURST 2
 /**
- * REALTIME_PRIORITY_NEXT_ROUND: The processor control is transfered to the thread
- * in the next round and the thread is delayed until all remaining active threads
- * are run.
+ * REALTIME_PRIORITY_END_OF_ROUND: The processor control is transfered to the 
+ * thread in the end of the round and the thread is delayed until all remaining 
+ * active threads are run.
  */
-#define REALTIME_PRIORITY_NEXT_ROUND 3
+#define REALTIME_PRIORITY_END_OF_ROUND 3
 
 /**
  * This class models the concept of priority for the control based scheduler.
@@ -79,7 +79,7 @@ public:
      * \param priority the desired priority value.
      */
     ControlSchedulerPriority(short int priority): priority(priority), 
-            realtime(REALTIME_PRIORITY_NEXT_ROUND) {}
+            realtime(REALTIME_PRIORITY_END_OF_ROUND) {}
     
     ControlSchedulerPriority(short int priority, short int realtime):
             priority(priority),realtime(realtime){}
@@ -105,6 +105,18 @@ public:
         return this->priority>=0 && this->priority<PRIORITY_MAX && 
                 this->realtime >=1 && this->realtime<=3;
     }
+    
+    /**
+     * This function acts like a less-than operator but should be only used in
+     * synchronization module for priority inheritance. The concept of priority
+     * for preemption is not exactly the same for priority inheritance, hence,
+     * this operation is a branch out of preemption priority for inheritance
+     * purpose.
+     * @return 
+     */
+    inline bool mutexLessOp(ControlSchedulerPriority b){
+        return false;
+    }
 
 private:
     short int priority;///< The priority value
@@ -113,32 +125,21 @@ private:
 
 inline bool operator <(ControlSchedulerPriority a, ControlSchedulerPriority b)
 {
-    return a.getRealtime() < b.getRealtime();
+    return b.getRealtime() == 1 && a.getRealtime() != 1;
 }
 
-inline bool operator <=(ControlSchedulerPriority a, ControlSchedulerPriority b)
-{
-    return a.get() <= b.get();
-}
-
-inline bool operator >(ControlSchedulerPriority a, ControlSchedulerPriority b)
-{
-    return a.get() > b.get();
-}
-
-inline bool operator >=(ControlSchedulerPriority a, ControlSchedulerPriority b)
-{
-    return a.get() >= b.get();
+inline bool operator>(ControlSchedulerPriority a, ControlSchedulerPriority b){
+    return a.getRealtime() == 1 && b.getRealtime() != 1;
 }
 
 inline bool operator ==(ControlSchedulerPriority a, ControlSchedulerPriority b)
 {
-    return a.get() == b.get();
+    return a.getRealtime() == b.getRealtime();
 }
 
 inline bool operator !=(ControlSchedulerPriority a, ControlSchedulerPriority b)
 {
-    return a.get() != b.get();
+    return a.getRealtime() != b.getRealtime();
 }
 
 struct ThreadsListItem : public IntrusiveListItem
diff --git a/miosix/kernel/scheduler/priority/priority_scheduler.cpp b/miosix/kernel/scheduler/priority/priority_scheduler.cpp
index f98dc625..1ecefc64 100644
--- a/miosix/kernel/scheduler/priority/priority_scheduler.cpp
+++ b/miosix/kernel/scheduler/priority/priority_scheduler.cpp
@@ -37,9 +37,12 @@ namespace miosix {
 //These are defined in kernel.cpp
 extern volatile Thread *cur;
 extern volatile int kernel_running;
-static ContextSwitchTimer& timer = ContextSwitchTimer::instance();
 extern IntrusiveList<SleepData> *sleepingList;
 
+//Internal data
+static ContextSwitchTimer& timer = ContextSwitchTimer::instance();
+static long long nextPeriodicPreemption = std::numeric_limits<long long>::max();
+
 //
 // class PriorityScheduler
 //
@@ -200,16 +203,23 @@ void PriorityScheduler::IRQsetIdleThread(Thread *idleThread)
     idle=idleThread;
 }
 
-static void setNextPreemption(bool curIsIdleThread){
+long long PriorityScheduler::IRQgetNextPreemption()
+{
+    return nextPeriodicPreemption;
+}
+
+static void IRQsetNextPreemption(bool curIsIdleThread){
     long long firstWakeupInList;
     if (sleepingList->empty())
         firstWakeupInList = std::numeric_limits<long long>::max();
     else
         firstWakeupInList = sleepingList->front()->wakeup_time;
+    
     if (curIsIdleThread){
         timer.IRQsetNextInterrupt(firstWakeupInList);
+        nextPeriodicPreemption = std::numeric_limits<long long>::max();
     }else{
-        long long nextPeriodicPreemption = timer.IRQgetCurrentTime() + preemptionPeriodNs;   
+        nextPeriodicPreemption = timer.IRQgetCurrentTime() + preemptionPeriodNs;   
         if (firstWakeupInList < nextPeriodicPreemption )
             timer.IRQsetNextInterrupt(firstWakeupInList);
         else
@@ -246,7 +256,7 @@ unsigned int PriorityScheduler::IRQfindNextThread()
                 //Rotate to next thread so that next time the list is walked
                 //a different thread, if available, will be chosen first
                 thread_list[i]=temp;
-                setNextPreemption(false);
+                IRQsetNextPreemption(false);
                 return preemptionPeriodNs;
             } else temp=temp->schedData.next;
             if(temp==thread_list[i]->schedData.next) break;
@@ -258,7 +268,7 @@ unsigned int PriorityScheduler::IRQfindNextThread()
     #ifdef WITH_PROCESSES
     MPUConfiguration::IRQdisable();
     #endif //WITH_PROCESSES
-    setNextPreemption(true);
+    IRQsetNextPreemption(true);
     return preemptionPeriodNs;
 }
 
diff --git a/miosix/kernel/scheduler/priority/priority_scheduler.h b/miosix/kernel/scheduler/priority/priority_scheduler.h
index 24b2a3cf..e4fa55b7 100755
--- a/miosix/kernel/scheduler/priority/priority_scheduler.h
+++ b/miosix/kernel/scheduler/priority/priority_scheduler.h
@@ -139,6 +139,8 @@ public:
      * \return the burst time
      */
     static unsigned int IRQfindNextThread();
+    
+    static long long IRQgetNextPreemption();
 
 private:
 
diff --git a/miosix/kernel/scheduler/priority/priority_scheduler_types.h b/miosix/kernel/scheduler/priority/priority_scheduler_types.h
index e58880c9..51b40d16 100644
--- a/miosix/kernel/scheduler/priority/priority_scheduler_types.h
+++ b/miosix/kernel/scheduler/priority/priority_scheduler_types.h
@@ -70,6 +70,18 @@ public:
     {
         return this->priority>=0 && this->priority<PRIORITY_MAX;
     }
+    
+    /**
+     * This function acts like a less-than operator but should be only used in
+     * synchronization module for priority inheritance. The concept of priority
+     * for preemption is not exactly the same for priority inheritance, hence,
+     * this operation is a branch out of preemption priority for inheritance
+     * purpose.
+     * @return 
+     */
+    inline bool mutexLessOp(PrioritySchedulerPriority b){
+        return priority < b.priority;
+    }
 
 private:
     short int priority;///< The priority value
@@ -80,21 +92,11 @@ inline bool operator <(PrioritySchedulerPriority a, PrioritySchedulerPriority b)
     return a.get() < b.get();
 }
 
-inline bool operator <=(PrioritySchedulerPriority a, PrioritySchedulerPriority b)
-{
-    return a.get() <= b.get();
-}
-
 inline bool operator >(PrioritySchedulerPriority a, PrioritySchedulerPriority b)
 {
     return a.get() > b.get();
 }
 
-inline bool operator >=(PrioritySchedulerPriority a, PrioritySchedulerPriority b)
-{
-    return a.get() >= b.get();
-}
-
 inline bool operator ==(PrioritySchedulerPriority a, PrioritySchedulerPriority b)
 {
     return a.get() == b.get();
diff --git a/miosix/kernel/scheduler/scheduler.h b/miosix/kernel/scheduler/scheduler.h
index 047bc49f..cb4043ed 100755
--- a/miosix/kernel/scheduler/scheduler.h
+++ b/miosix/kernel/scheduler/scheduler.h
@@ -169,7 +169,17 @@ public:
     {
         return T::IRQfindNextThread();
     }
-
+    
+    /**
+     * It returns the next preemption to be caused by the scheduler
+     * i.e. the beginning of the next burst.
+     * At the beginning, when no burst exists it returns
+     * numeric_limits<long long>::max().
+     */
+    static long long IRQgetNextPreemption()
+    {
+        return T::IRQgetNextPreemption();
+    }
 };
 
 #ifdef SCHED_TYPE_PRIORITY
diff --git a/miosix/kernel/scheduler/timer_interrupt.h b/miosix/kernel/scheduler/timer_interrupt.h
index 7de8fd6f..9615ca61 100644
--- a/miosix/kernel/scheduler/timer_interrupt.h
+++ b/miosix/kernel/scheduler/timer_interrupt.h
@@ -48,9 +48,12 @@ extern bool IRQwakeThreads(long long currentTick);///\internal Do not use outsid
 inline void IRQtimerInterrupt(long long currentTick)
 {
     miosix_private::IRQstackOverflowCheck();
-    IRQwakeThreads(currentTick);
+    bool hptw = IRQwakeThreads(currentTick);
+    if (currentTick >= Scheduler::IRQgetNextPreemption() || hptw ){
+        //End of the burst || a higher priority thread has woken up
+        Scheduler::IRQfindNextThread();//If the kernel is running, preempt
+    }
     
-    Scheduler::IRQfindNextThread();//If the kernel is running, preempt
     if(kernel_running!=0) tick_skew=true;
     
     #ifndef SCHED_TYPE_PRIORITY
@@ -59,7 +62,7 @@ inline void IRQtimerInterrupt(long long currentTick)
     //Now we can have no periodic preemption, and so if we are called (and EDF
     //is selected), at least one thread has woken up, so there's no need for woken
     //Modify the code below accordingly
-    #error "FIXME"
+    //#error "FIXME"
     #endif
 
 //    miosix_private::IRQstackOverflowCheck();
diff --git a/miosix/kernel/sync.cpp b/miosix/kernel/sync.cpp
index 733e7202..9cb00348 100644
--- a/miosix/kernel/sync.cpp
+++ b/miosix/kernel/sync.cpp
@@ -81,7 +81,7 @@ void Mutex::PKlock(PauseKernelLock& dLock)
     //Handle priority inheritance
     if(p->mutexWaiting!=0) errorHandler(UNEXPECTED);
     p->mutexWaiting=this;
-    if(p->getPriority()>owner->getPriority())
+    if (owner->getPriority().mutexLessOp(p->getPriority()))
     {
         Thread *walk=owner;
         for(;;)
@@ -150,7 +150,7 @@ void Mutex::PKlockToDepth(PauseKernelLock& dLock, unsigned int depth)
     //Handle priority inheritance
     if(p->mutexWaiting!=0) errorHandler(UNEXPECTED);
     p->mutexWaiting=this;
-    if(p->getPriority()>owner->getPriority())
+    if (owner->getPriority().mutexLessOp(p->getPriority()))
     {
         Thread *walk=owner;
         for(;;)
@@ -250,7 +250,8 @@ bool Mutex::PKunlock(PauseKernelLock& dLock)
         while(walk!=0)
         {
             if(walk->waiting.empty()==false)
-                pr=max(pr,walk->waiting.front()->getPriority());
+                if (pr.mutexLessOp(walk->waiting.front()->getPriority()))
+                    pr = walk->waiting.front()->getPriority();
             walk=walk->next;
         }
         if(pr!=owner->getPriority()) Scheduler::PKsetPriority(owner,pr);
@@ -273,9 +274,9 @@ bool Mutex::PKunlock(PauseKernelLock& dLock)
         owner->mutexLocked=this;
         //Handle priority inheritance of new owner
         if(waiting.empty()==false &&
-           waiting.front()->getPriority()>owner->getPriority())
+                owner->getPriority().mutexLessOp(waiting.front()->getPriority()))
                 Scheduler::PKsetPriority(owner,waiting.front()->getPriority());
-        return owner->getPriority() > p->getPriority();
+        return p->getPriority().mutexLessOp(owner->getPriority());
     } else {
         owner=0; //No threads waiting
         std::vector<Thread *>().swap(waiting); //Save some RAM
@@ -321,7 +322,8 @@ unsigned int Mutex::PKunlockAllDepthLevels(PauseKernelLock& dLock)
         while(walk!=0)
         {
             if(walk->waiting.empty()==false)
-                pr=max(pr,walk->waiting.front()->getPriority());
+                if (pr.mutexLessOp(walk->waiting.front()->getPriority()))
+                    pr = walk->waiting.front()->getPriority();
             walk=walk->next;
         }
         if(pr!=owner->getPriority()) Scheduler::PKsetPriority(owner,pr);
@@ -344,7 +346,7 @@ unsigned int Mutex::PKunlockAllDepthLevels(PauseKernelLock& dLock)
         owner->mutexLocked=this;
         //Handle priority inheritance of new owner
         if(waiting.empty()==false &&
-           waiting.front()->getPriority()>owner->getPriority())
+                owner->getPriority().mutexLessOp(waiting.front()->getPriority()))
                 Scheduler::PKsetPriority(owner,waiting.front()->getPriority());
     } else {
         owner=0; //No threads waiting
@@ -430,8 +432,8 @@ void ConditionVariable::signal()
         //Wakeup
         first->p->flags.IRQsetCondWait(false);
         //Check for priority issues
-        if(first->p->IRQgetPriority() >
-                Thread::IRQgetCurrentThread()->IRQgetPriority()) hppw=true;
+        if(Thread::IRQgetCurrentThread()->IRQgetPriority() <
+                first->p->IRQgetPriority()) hppw=true;
         //Remove from list
         first=first->next;
     }
@@ -452,8 +454,8 @@ void ConditionVariable::broadcast()
             //Wakeup
             first->p->flags.IRQsetCondWait(false);
             //Check for priority issues
-            if(first->p->IRQgetPriority() >
-                Thread::IRQgetCurrentThread()->IRQgetPriority()) hppw=true;
+            if(Thread::IRQgetCurrentThread()->IRQgetPriority() <
+                first->p->IRQgetPriority()) hppw=true;
             //Remove from list
             first=first->next;
         }
-- 
GitLab