| Re: [pok-devel] Problem with the POK scheduler... |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/pok-devel Archives
]
Hello everybody
the attached patch fixes the problem I have seen in the scheduler. It validates the test suite and has no known problems
however since it deals in the intricacies of the scheduler I would like to have as much review/testing as possible
the problem was that the current_partition global variable was changed too early.
the new philosophy is that elect_partition will not change the global state, nor will elect_thread.
thus, when elect_thread is called, current_partition and current_thread are still set to the old thread/partition.
All changes are done after the decisions are taken.
Please review and feedback so I can commit this changes soon
Regards
Jérémy Rosen
fight key loggers : write some perl using vim
----- Mail original -----
> Hello everybody
>
> while debugging a problem with partition initialisation I stumbled on
> a limitation in the POK scheduler that might be a serious bug... I
> am trying to find all the ins and outs of that bug but I would
> likely take any advice...
>
>
>
> THE SYMPTOMS
>
> I have two partitions (A and B) with main threads (Amain and Bmain)
>
> A is started and runs Amain
> - end of window, Amain isn't done yet
> B is started and runs Bmain
> Bmain set the partition mode to normal
>
> Bug : immediately Amain starts axecuting again within B's time slot
> (which isn't over yet)
>
>
> WHAT IS ACTUALLY HAPPENING
>
> when Amain is executing and is swapped out, it is stored in
> A.current_thread which is the IDLE_THREAD at this point. As a
> consequence, when Bmain is done, it runs the idle thread which
> contains Amain's code
>
> The problem is that pok_sched_elect_partition does the actual
> partition switching and that confuses the pok_sched_elect_thread
> function.
>
> the pok_sched_elect_thread function seems to only work properly when
> a change of partition occurs only if the partition that is being
> left is running the idle thread.
>
>
> so...
>
>
> * is this a design choice ?
> * has anybody encountered that ?
> * any advice on fixing it ?
>
> I am working on a fix but it's complicated and touches the whole
> scheduler logic (mainly because of unsafe assumptions with
> current_thread and prev_current_thread which are assumed to be part
> of the current partition) so any word of wisdom would be welcome
>
> Regards
>
> Jérémy Rosen
>
> fight key loggers : write some perl using vim
>
>
>
diff --git a/pok/trunk/kernel/core/debug.c b/pok/trunk/kernel/core/debug.c
index f218758..f28b4e3 100644
--- a/pok/trunk/kernel/core/debug.c
+++ b/pok/trunk/kernel/core/debug.c
@@ -54,7 +54,7 @@ void pok_debug_print_current_state ()
printf ("Base vaddr : 0x%x\n", POK_CURRENT_PARTITION.base_vaddr);
printf ("Size : %d\n", POK_CURRENT_PARTITION.size);
printf ("Current thread : %d\n", POK_CURRENT_PARTITION.current_thread);
- printf ("Prev current thread : %d\n", POK_CURRENT_PARTITION.prev_current_thread);
+ printf ("Prev current thread : %d\n", POK_CURRENT_PARTITION.prev_thread);
printf ("Main thread : %d\n", POK_CURRENT_PARTITION.thread_main);
printf ("Main thread entry : 0x%x\n", POK_CURRENT_PARTITION.thread_main_entry);
printf ("Partition threads sp :");
diff --git a/pok/trunk/kernel/core/partition.c b/pok/trunk/kernel/core/partition.c
index dc9ed5d..255f67b 100644
--- a/pok/trunk/kernel/core/partition.c
+++ b/pok/trunk/kernel/core/partition.c
@@ -97,7 +97,7 @@ void pok_partition_reinit (const uint8_t pid)
pok_partitions[pid].thread_index = 0;
pok_partitions[pid].current_thread = pok_partitions[pid].thread_index_low;
- pok_partitions[pid].prev_current_thread = IDLE_THREAD;
+ pok_partitions[pid].prev_thread = IDLE_THREAD; // breaks the rule of prev_thread not being idle, but it's just for init
#ifdef POK_NEEDS_ERROR_HANDLING
pok_partitions[pid].thread_error = 0;
@@ -200,7 +200,7 @@ pok_ret_t pok_partition_init ()
pok_partitions[i].thread_index = 0;
pok_partitions[i].thread_main = 0;
pok_partitions[i].current_thread = IDLE_THREAD;
- pok_partitions[i].prev_current_thread = IDLE_THREAD;
+ pok_partitions[i].prev_thread = IDLE_THREAD; // breaks the rule of prev_thread not being idle, but it's just for init
#ifdef POK_NEEDS_SCHED_HFPPS
pok_partitions[i].payback = 0;
@@ -241,6 +241,7 @@ pok_ret_t pok_partition_init ()
#endif
pok_partition_setup_main_thread (i);
+ pok_partitions[i].current_thread = pok_partitions[i].thread_main;
}
return POK_ERRNO_OK;
diff --git a/pok/trunk/kernel/core/sched.c b/pok/trunk/kernel/core/sched.c
index cd93d57..d550253 100644
--- a/pok/trunk/kernel/core/sched.c
+++ b/pok/trunk/kernel/core/sched.c
@@ -65,7 +65,6 @@ uint64_t pok_sched_slots[POK_CONFIG_SCHEDULING_NBSLOTS]
uint8_t pok_sched_slots_allocation[POK_CONFIG_SCHEDULING_NBSLOTS]
= (uint8_t[]) POK_CONFIG_SCHEDULING_SLOTS_ALLOCATION;
-uint32_t prev_current_thread;
pok_sched_t pok_global_sched;
uint64_t pok_sched_next_deadline;
uint64_t pok_sched_next_major_frame;
@@ -129,8 +128,9 @@ uint8_t pok_sched_get_priority_max (const pok_sched_t sched_type)
}
#ifdef POK_NEEDS_PARTITIONS
-pok_partition_t* pok_elect_partition()
+uint8_t pok_elect_partition()
{
+ uint8_t next_partition = POK_SCHED_CURRENT_PARTITION;
# if POK_CONFIG_NB_PARTITIONS > 1
uint64_t now = POK_GETTICK();
@@ -151,39 +151,33 @@ pok_partition_t* pok_elect_partition()
* FIXME : current debug session about exceptions-handled
printf ("Switch from partition %d to partition %d\n", pok_current_partition, pok_sched_current_slot);
printf ("old current thread = %d\n", POK_SCHED_CURRENT_THREAD);
- printf ("old prev current thread = %d\n", prev_current_thread);
printf ("new current thread = %d\n", pok_partitions[pok_sched_current_slot].current_thread);
- printf ("new prev current thread = %d\n", pok_partitions[pok_sched_current_slot].prev_current_thread);
+ printf ("new prev current thread = %d\n", pok_partitions[pok_sched_current_slot].prev_thread);
*/
- pok_partitions[pok_current_partition].prev_current_thread = prev_current_thread;
- pok_partitions[pok_current_partition].current_thread = POK_SCHED_CURRENT_THREAD;
-
- pok_current_partition = pok_sched_slots_allocation[pok_sched_current_slot];
-
- prev_current_thread = pok_partitions[pok_current_partition].prev_current_thread;
- current_thread = pok_partitions[pok_current_partition].current_thread;
+ next_partition = pok_sched_slots_allocation[pok_sched_current_slot];
#ifdef POK_NEEDS_SCHED_HFPPS
- if (pok_partitions[pok_current_partition].payback > 0) // pay back!
+ if (pok_partitions[next_partition].payback > 0) // pay back!
{
// new deadline
- pok_sched_next_deadline -= pok_partitions[pok_current_partition].payback;
- pok_partitions[pok_current_partition].payback = 0;
+ pok_sched_next_deadline -= pok_partitions[next_partition].payback;
+ pok_partitions[next_partition].payback = 0;
}
#endif /* POK_NEEDS_SCHED_HFPPS */
}
# endif /* POK_CONFIG_NB_PARTITIONS > 1 */
- return (&(pok_partitions[pok_current_partition]));
+ return next_partition;
}
#endif /* POK_NEEDS_PARTITIONS */
#ifdef POK_NEEDS_PARTITIONS
-uint32_t pok_elect_thread(pok_partition_t* current_partition)
+uint32_t pok_elect_thread(uint8_t new_partition_id)
{
uint64_t now = POK_GETTICK();
+ pok_partition_t* new_partition = &(pok_partitions[new_partition_id]);
/*
@@ -194,9 +188,9 @@ uint32_t pok_elect_thread(pok_partition_t* current_partition)
* type
*/
pok_thread_t* thread;
- for (i = 0; i < pok_partitions[pok_current_partition].nthreads; i++)
+ for (i = 0; i < new_partition->nthreads; i++)
{
- thread = &(pok_threads[current_partition->thread_index_low + i]);
+ thread = &(pok_threads[new_partition->thread_index_low + i]);
#if defined (POK_NEEDS_LOCKOBJECTS) || defined (POK_NEEDS_PORTS_QUEUEING) || defined (POK_NEEDS_PORTS_SAMPLING)
if ((thread->state == POK_STATE_WAITING) && (thread->wakeup_time <= now))
@@ -217,32 +211,31 @@ uint32_t pok_elect_thread(pok_partition_t* current_partition)
* We elect the thread to be executed.
*/
uint32_t elected;
- switch (current_partition->mode)
+ switch (new_partition->mode)
{
case POK_PARTITION_MODE_INIT_COLD:
case POK_PARTITION_MODE_INIT_WARM:
#ifdef POK_NEEDS_ERROR_HANDLING
- if ((current_partition->thread_error != 0) &&
- (pok_threads[current_partition->thread_error].state != POK_STATE_STOPPED))
+ if ((new_partition->thread_error != 0) &&
+ (pok_threads[new_partition->thread_error].state != POK_STATE_STOPPED))
{
- elected = current_partition->thread_error;
+ elected = new_partition->thread_error;
}
else
{
- elected = current_partition->thread_main;
+ elected = new_partition->thread_main;
}
#endif
- elected = current_partition->thread_main;
+ elected = new_partition->thread_main;
break;
case POK_PARTITION_MODE_NORMAL:
#ifdef POK_NEEDS_ERROR_HANDLING
- if ((POK_SCHED_CURRENT_THREAD == POK_CURRENT_PARTITION.thread_error) &&
- (POK_CURRENT_THREAD.state == POK_STATE_RUNNABLE))
+ if ((new_partition->current_thread == new_partition->thread_error) &&
+ (pok_threads[new_partition->current_thread].state == POK_STATE_RUNNABLE))
{
- elected = POK_CURRENT_PARTITION.thread_error;
- POK_CURRENT_PARTITION.current_thread = elected;
+ elected = new_partition->thread_error;
break;
}
#endif
@@ -262,15 +255,16 @@ uint32_t pok_elect_thread(pok_partition_t* current_partition)
POK_CURRENT_THREAD.state = POK_STATE_WAIT_NEXT_ACTIVATION;
}
}
- elected = POK_CURRENT_PARTITION.sched_func (current_partition->thread_index_low,
- current_partition->thread_index_high);
+ elected = new_partition->sched_func (new_partition->thread_index_low,
+ new_partition->thread_index_high,
+ new_partition->prev_thread,
+ new_partition->current_thread);
#ifdef POK_NEEDS_INSTRUMENTATION
- if ( (elected != IDLE_THREAD) && (elected != POK_CURRENT_PARTITION.thread_main))
+ if ( (elected != IDLE_THREAD) && (elected != new_partition->thread_main))
{
pok_instrumentation_running_task (elected);
}
#endif
- POK_CURRENT_PARTITION.current_thread = elected;
break;
@@ -298,6 +292,7 @@ uint32_t pok_elect_thread(pok_partition_t* current_partition)
void pok_sched()
{
uint32_t elected_thread = 0;
+ uint8_t elected_partition = POK_SCHED_CURRENT_PARTITION;
#ifdef POK_NEEDS_SCHED_HFPPS
uint64_t now = POK_GETTICK();
@@ -315,10 +310,18 @@ void pok_sched()
else /* overmegadirty */
#endif /* POK_NEEDS_SCHED_HFPPS */
{
- pok_partition_t* elected_partition = pok_elect_partition();
+
+ elected_partition = pok_elect_partition();
elected_thread = pok_elect_thread(elected_partition);
}
+ pok_current_partition = elected_partition;
+ if(pok_partitions[pok_current_partition].current_thread != elected_thread) {
+ if(pok_partitions[pok_current_partition].current_thread != IDLE_THREAD) {
+ pok_partitions[pok_current_partition].prev_thread = pok_partitions[pok_current_partition].current_thread;
+ }
+ pok_partitions[pok_current_partition].current_thread = elected_thread;
+ }
pok_sched_context_switch(elected_thread);
}
#else
@@ -360,6 +363,7 @@ void pok_sched_context_switch (const uint32_t elected_id)
{
return;
}
+
current_sp = &POK_CURRENT_THREAD.sp;
new_sp = pok_threads[elected_id].sp;
/*
@@ -376,24 +380,12 @@ void pok_sched_context_switch (const uint32_t elected_id)
}
#ifdef POK_NEEDS_SCHED_RMS
-uint32_t pok_sched_part_rms (const uint32_t index_low, const uint32_t index_high)
+uint32_t pok_sched_part_rms (const uint32_t index_low, const uint32_t index_high,const uint32_t __attribute__((unused)) prev_thread,const uint32_t __attribute__((unused)) current_thread)
{
uint32_t res;
#ifdef POK_NEEDS_DEBUG
uint32_t from;
-#endif
-
- if (POK_SCHED_CURRENT_THREAD == IDLE_THREAD)
- {
- res = prev_current_thread;
- }
- else
- {
- res = POK_SCHED_CURRENT_THREAD;
- }
-
-#ifdef POK_NEEDS_DEBUG
- from = res;
+ from = prev_thread;
#endif
res= index_low;
@@ -412,10 +404,6 @@ uint32_t pok_sched_part_rms (const uint32_t index_low, const uint32_t index_high
if ((res == index_low) && (pok_threads[res].state != POK_STATE_RUNNABLE))
{
res = IDLE_THREAD;
- if (POK_SCHED_CURRENT_THREAD != IDLE_THREAD)
- {
- prev_current_thread = POK_SCHED_CURRENT_THREAD;
- }
}
#ifdef POK_NEEDS_DEBUG
@@ -441,25 +429,25 @@ uint32_t pok_sched_part_rms (const uint32_t index_low, const uint32_t index_high
#endif /* POK_NEEDS_SCHED_RMS */
-uint32_t pok_sched_part_rr (const uint32_t index_low, const uint32_t index_high)
+uint32_t pok_sched_part_rr (const uint32_t index_low, const uint32_t index_high,const uint32_t prev_thread,const uint32_t current_thread)
{
uint32_t res;
uint32_t from;
- if (POK_SCHED_CURRENT_THREAD == IDLE_THREAD)
+ if (current_thread == IDLE_THREAD)
{
- res = prev_current_thread;
+ res = prev_thread;
}
else
{
- res = POK_SCHED_CURRENT_THREAD;
+ res = current_thread;
}
from = res;
- if ((POK_CURRENT_THREAD.remaining_time_capacity > 0) && (POK_CURRENT_THREAD.state == POK_STATE_RUNNABLE))
+ if ((pok_threads[current_thread].remaining_time_capacity > 0) && (pok_threads[current_thread].state == POK_STATE_RUNNABLE))
{
- return POK_SCHED_CURRENT_THREAD;
+ return current_thread;
}
do
@@ -475,10 +463,6 @@ uint32_t pok_sched_part_rr (const uint32_t index_low, const uint32_t index_high)
if ((res == from) && (pok_threads[res].state != POK_STATE_RUNNABLE))
{
res = IDLE_THREAD;
- if (POK_SCHED_CURRENT_THREAD != IDLE_THREAD)
- {
- prev_current_thread = POK_SCHED_CURRENT_THREAD;
- }
}
return res;
}
diff --git a/pok/trunk/kernel/include/core/partition.h b/pok/trunk/kernel/include/core/partition.h
index 93e7bc0..40e85e2 100644
--- a/pok/trunk/kernel/include/core/partition.h
+++ b/pok/trunk/kernel/include/core/partition.h
@@ -82,10 +82,10 @@ typedef struct
pok_sched_t sched; /**< The associated for the partition to schedule its threads */
- uint32_t (*sched_func)(uint32_t low, uint32_t high); /**< Scheduling function to scheduler threads */
+ uint32_t (*sched_func)(uint32_t low, uint32_t high,uint32_t prev_thread, uint32_t cur_thread); /**< Scheduling function to scheduler threads */
uint64_t activation; /**< Last activation time of the partition */
- uint32_t prev_current_thread; /**< member for the scheduler (previous scheduled thread inside the partition */
+ uint32_t prev_thread; /**< member for the scheduler (previous scheduled real thread inside the partition,i.e not the idle thread */
uint32_t current_thread; /**< member for the scheduler (current executed thread inside the partition */
uint32_t thread_index_low; /**< The low index in the threads table */
diff --git a/pok/trunk/kernel/include/core/sched.h b/pok/trunk/kernel/include/core/sched.h
index 4180e5e..80432b2 100644
--- a/pok/trunk/kernel/include/core/sched.h
+++ b/pok/trunk/kernel/include/core/sched.h
@@ -54,8 +54,8 @@ uint8_t pok_sched_get_priority_max (const pok_sched_t sched_type);
/* Scheduler election method */
uint8_t pok_sched_election (void);
-uint32_t pok_sched_part_rr (const uint32_t ,const uint32_t);
-uint32_t pok_sched_part_rms (const uint32_t ,const uint32_t);
+uint32_t pok_sched_part_rr (const uint32_t ,const uint32_t,const uint32_t prev_thread,const uint32_t current_thread);
+uint32_t pok_sched_part_rms (const uint32_t ,const uint32_t,const uint32_t prev_thread,const uint32_t current_thread);
/* Context switch functions */
void pok_sched_context_switch (const uint32_t);