Re: [riot-devel] How do ISRs work in the context of RIOT's scheduler?

2016-11-29 Thread Oleg Hahm
Hi Kaspar!

On Tue, Nov 29, 2016 at 11:59:14AM +0100, Kaspar Schleiser wrote:
> On 11/29/2016 11:14 AM, Oleg Hahm wrote:
> > Okay, re-reading the documentation again, I agree that the behavior deviates
> > from what is written. I also see, that the behavior is indeed different on
> > different architectures which is clearly a bug.
> 
> Turns out this is done similarily on all platforms...

Except MSP430, ARM7, native, and x86, you mean? ;-)

Cheers,
Oleg
-- 
printk(KERN_ERR "%s: Something Wicked happened! %4.4x.\n",...);
linux-2.6.6/drivers/net/sundance.c


signature.asc
Description: PGP signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] How do ISRs work in the context of RIOT's scheduler?

2016-11-29 Thread Kaspar Schleiser

Hi,

On 11/29/2016 11:14 AM, Oleg Hahm wrote:

Okay, re-reading the documentation again, I agree that the behavior deviates
from what is written. I also see, that the behavior is indeed different on
different architectures which is clearly a bug.


Turns out this is done similarily on all platforms...
I've PR'ed a fix in [1].

Kaspar
[1] https://github.com/RIOT-OS/RIOT/pull/6164
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] How do ISRs work in the context of RIOT's scheduler?

2016-11-29 Thread Oleg Hahm
Hi Kaspar!

On Tue, Nov 29, 2016 at 10:41:11AM +0100, Kaspar Schleiser wrote:
> On 11/29/2016 10:27 AM, Oleg Hahm wrote:
> > I would call it rather a bug in the documentation. The behavior of the
> > scheduler for threads of the same priority is simply underspecified.
> 
> We had that discussion offline. The "bug" is both in Cortex-M's ISR
> handling and in the docs.

Okay, re-reading the documentation again, I agree that the behavior deviates
from what is written. I also see, that the behavior is indeed different on
different architectures which is clearly a bug.

> > "Assigning the same priority to two or more threads is usually not a good
> > idea. A thread in RIOT may run until it yields (thread_yield) or another
> > thread with higher priority is runnable (STATUS_ON_RUNQUEUE) again. Multiple
> > threads with the same priority will therefore be scheduled cooperatively: 
> > when
> > one of them is running, all others with the same priority depend on it to
> > yield (or be interrupted by a thread with higher priority).
> 
> This last half sentence (in braces) is wrong. A higher thread might
> interrupt one with "siblings" of the same priority, but after the
> high-prio thread is done, the scheduler would switch back to the initial
> thread.

That's probably my fault. I always thought that `sched_run()` would advance
the pointer which is apparently not true.

Cheers,
Oleg
-- 
The great thing about SQL transaction jokes is that once you BEGIN, you can
ROLLBACK if nobody gets them.


signature.asc
Description: PGP signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] How do ISRs work in the context of RIOT's scheduler?

2016-11-29 Thread Kaspar Schleiser
Hi,

On 11/29/2016 10:27 AM, Oleg Hahm wrote:
> I would call it rather a bug in the documentation. The behavior of the
> scheduler for threads of the same priority is simply underspecified.

We had that discussion offline. The "bug" is both in Cortex-M's ISR
handling and in the docs.

> "Assigning the same priority to two or more threads is usually not a good
> idea. A thread in RIOT may run until it yields (thread_yield) or another
> thread with higher priority is runnable (STATUS_ON_RUNQUEUE) again. Multiple
> threads with the same priority will therefore be scheduled cooperatively: when
> one of them is running, all others with the same priority depend on it to
> yield (or be interrupted by a thread with higher priority).

This last half sentence (in braces) is wrong. A higher thread might
interrupt one with "siblings" of the same priority, but after the
high-prio thread is done, the scheduler would switch back to the initial
thread.

We should re-word the whole paragraph.

> Hence, if we change the scheduler and update the documentation accordingly, we
> should also make sure that the pointer in the list of threads with equal
> priority is not advanced if a thread gets interrupted by a thread with a
> higher priority, either. I.e., the pointer should only be advanced if a thread
> explicitly calls `thread_yield()`.

That is already the case. thread_yield() advances the current priority's
circular list, then calls thread_yield_higher().
But the ISR handling should directly call thread_yield_higher() instead
of thread_yield(), as Charles pointed out.

Kaspar



signature.asc
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] How do ISRs work in the context of RIOT's scheduler?

2016-11-29 Thread Oleg Hahm
Hi Kaspar!

On Tue, Nov 29, 2016 at 08:46:58AM +0100, Kaspar Schleiser wrote:
> On 11/26/2016 12:55 AM, Charles Cross wrote:
> > Perhaps this is designed for the case where actions are taken inside the
> > ISR which signal other threads (like sending IPC messages), but I'm
> > still not clear on why it would need to reschedule the previously
> > executing thread relative to other running or pending threads of equal
> > priority.
> 
> Your analysis is correct, and you might have found a bug in the
> scheduling logic.

I would call it rather a bug in the documentation. The behavior of the
scheduler for threads of the same priority is simply underspecified. The only
thing the API says here is:

"Assigning the same priority to two or more threads is usually not a good
idea. A thread in RIOT may run until it yields (thread_yield) or another
thread with higher priority is runnable (STATUS_ON_RUNQUEUE) again. Multiple
threads with the same priority will therefore be scheduled cooperatively: when
one of them is running, all others with the same priority depend on it to
yield (or be interrupted by a thread with higher priority). This may make it
difficult to determine when which of them gets scheduled and how much CPU time
they will get. In most applications, the number of threads in application is
significantly smaller than the number of available priorities, so assigning
distinct priorities per thread should not be a problem. Only assign the same
priority to multiple threads if you know what you are doing!"
[Source: https://riot-os.org/api/group__core__thread.html]

Hence, if we change the scheduler and update the documentation accordingly, we
should also make sure that the pointer in the list of threads with equal
priority is not advanced if a thread gets interrupted by a thread with a
higher priority, either. I.e., the pointer should only be advanced if a thread
explicitly calls `thread_yield()`.

Cheers,
Oleg
-- 
What's a pirate's favorite method of remote access? - ADP


signature.asc
Description: PGP signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] How do ISRs work in the context of RIOT's scheduler?

2016-11-28 Thread Kaspar Schleiser
Hi Charles,

On 11/26/2016 12:55 AM, Charles Cross wrote:
> Perhaps this is designed for the case where actions are taken inside the
> ISR which signal other threads (like sending IPC messages), but I'm
> still not clear on why it would need to reschedule the previously
> executing thread relative to other running or pending threads of equal
> priority.

Your analysis is correct, and you might have found a bug in the
scheduling logic. "sched_context_switch_request" is used by
"context-switch triggering" functions (sending a message, unblocking a
mutex), so after the ISR ends, the context switching code runs the
scheduler and then switches to the highest priority thread in pending state.
It should not advance the circular list of the current thread.
Threads in one priority level are supposed to be scheduled cooperatively.

So in essence, "thread_yield_higher()" should be called instead of
"thread_yield()".

I'll investigate some more and then probably fix this.

Thanks for digging this deep!

Kaspar
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


[riot-devel] How do ISRs work in the context of RIOT's scheduler?

2016-11-25 Thread Charles Cross
Hi all,

I'm rather new to using RIOT and have been hunting around for some thorough
documentation on how the scheduler works, but haven't really been able to
find anything that helps to explain what I am generally seeing in most ISR
definitions, which tend to end with the following bit of code:

if (sched_context_switch_request) {
thread_yield();
}

My first questions are: What are the general sources for context switch
requests and what does it mean to call thread_yield in an ISR? I would have
imagined that ISRs are called outside of the context of any thread, so
calling yield within them didn't immediately make sense to me. Is this just
a general pattern for triggering the scheduler to run, if necessary, after
the ISR has completed?

I walked through the code in thread_yield a bit and (for the Cortex-M
platform specifically) reached this partial understanding:

void thread_yield(void)
{
// Disable IRQs and store the PRIMASK
unsigned old_state = irq_disable();

// Get the currently active thread (In an ISR, I assume this is the
interrupted thread)
thread_t *me = (thread_t *)sched_active_thread;

// If the thread is currently running or pending
if (me->status >= STATUS_ON_RUNQUEUE) {
// Advance the next runnable thread at the current thread's
priority level
clist_lpoprpush(&sched_runqueues[me->priority]);
}

// Re-enable the original PRIMASK
irq_restore(old_state);

// Trigger the PendSV interrupt to run the scheduler
thread_yield_higher();
}

I'm having a bit of difficulty understanding the flow of events that would
lead you to call thread_yield inside of an ISR, as I imagine that it causes
the thread that was running before the ISR was executed to be the one that
yields. Why would it be necessary to cause that thread to yield to other
threads at the same level of priority simply because a potentially
unrelated ISR was run? I predict that the reasoning for that is explained
by how sched_context_switch_request operates.

Perhaps this is designed for the case where actions are taken inside the
ISR which signal other threads (like sending IPC messages), but I'm still
not clear on why it would need to reschedule the previously executing
thread relative to other running or pending threads of equal priority.

Any clarification or direction towards more indepth documentation of the
scheduling system is much appreciated.

Thanks much,
Charles
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel