[En-Nut-Discussion] NutSleep implementation optimization
Harald Kipp
harald.kipp at egnite.de
Mon Jun 13 20:03:16 CEST 2005
Matthias,
it's not final. Thus the docs hadn't been updated.
There are three remaining issues:
1. Not supporting callbacks within interrupt context breaks
some existing apps. At least Ole Reinhardt and Ralph Mason
stated this. On the other hand, most people voted for removal
of this support. However, if it's removed, do we need callbacks
at all? The kernel doesn't really, as it uses timers in two
ways only: Thread sleep and Event timeout.
2. Late timer release as it is implemented now in CVS HEAD may
cause memory overflow with some applications. A solution might be
to remove elapsed timers when creating new ones. _BUT_, in this
case thread switching has to be avoided. Threads do not expect
a context switch when starting a timer.
3. As posted in my answer to Oliver Schulz, setting runningTimer
to NULL to disable timer list processing in the interrupt handler
isn't that smart. It might be sufficient to simply disable
timer interrupts.
If we completely remove system timer callbacks, we need an
alternative - the new application timer API demanded by Ole.
If this new API supports timer callbacks in interrupt context,
these callbacks may again block the whole system.
Thus my question: Where's the advantage?
Regarding other critical sections: Yes, I think most of them if
not all can be removed. But I want to delay this until at least
issues 1 and 2 are solved.
Harald
At 18:53 13.06.2005 +0200, you wrote:
>Hi Harald,
>
>I'm just browsing to the NutTimerIrq changes. I really like the new
>IRQ handler. It's way cool/fast/elegant..
>
>Some things I stumbled upon:
>
>Docu to NutTimerStart writes: .. callback is executed in interrupt
>context..
>this isn't true anymore. so this comment could go away.
>
>But I found an Critical Section which I guess is still to long in the
>NutSleep implementation.
>If you all have a look at the NutSleep code with me:
>
>void NutSleep(u_long ms)
>{
> u_long ticks;
> if (ms) {
> ticks = NutTimerMillisToTicks(ms);
> NutEnterCritical();
> if ((runningThread->td_timer = NutTimerStartTicks(ticks,
>NutThreadWake, runningThread, TM_ONESHOT)) != 0) {
>#ifdef NUTDEBUG
> if (__os_trf) {
> static prog_char fmt1[] = "Rem<%p>\n";
> fprintf_P(__os_trs, fmt1, runningThread);
> }
>#endif
> NutThreadRemoveQueue(runningThread, &runQueue);
> runningThread->td_state = TDS_SLEEP;
>#ifdef NUTDEBUG
> if (__os_trf) {
> static prog_char fmt2[] = "SWS<%p %p>\n";
> NutDumpThreadList(__os_trs);
> //NutDumpThreadQueue(__os_trs, runQueue);
> fprintf_P(__os_trs, fmt2, runningThread, runQueue);
> }
>#endif
>#ifdef NUTTRACER
> TRACE_ADD_ITEM(TRACE_TAG_THREAD_SLEEP,(int)runningThread);
>#endif
> NutThreadResume();
> }
> NutExitCritical();
> } else
> NutThreadYield();
>}
>
>
>I'm wondering, a) if the critical section is needed and b) if yes,
>how it could be optimized?
>
>If the critical is needed, we see that the computation of ms to ticks
>is done outside, thats fine.
>But the NutTimerStartTicks includes a malloc operation and a
>potentially long NutTimerInsert
>function, which might block IRQ for too long.
>
>
>If I remember correctly, since the NutEventPostFromIrq modifications,
>the runningThread resp. runQueue
>is never changed from IRQ context anymore, hence doesn't need to be
>protected.
>Then: Removing of the runningThread from the runQueue shouldn't need
>protection either.
>
>Do I miss something? Could we just remove the Cirtical Section
>altogether (cool.. )?
>
>More on docu updates: If I'm right that thread queues are not
>modified from, then the
>comments on NutThreadAddPriQueue and NutThreadRemoveQueue stating
>"CPU interrupts must have been disabled before calling this function.
>" are obsolete?
>
>thanks for reading,
>
>matthias
>
>
>
>
>
>_______________________________________________
>En-Nut-Discussion mailing list
>En-Nut-Discussion at egnite.de
>http://www.egnite.de/mailman/listinfo.cgi/en-nut-discussion
More information about the En-Nut-Discussion
mailing list