NutSleep implementation optimization

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

NutSleep implementation optimization

Matthias Ringwald
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
[hidden email]
http://www.egnite.de/mailman/listinfo.cgi/en-nut-discussion
Reply | Threaded
Open this post in threaded view
|

Re: NutSleep implementation optimization

Harald Kipp
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
>[hidden email]
>http://www.egnite.de/mailman/listinfo.cgi/en-nut-discussion

_______________________________________________
En-Nut-Discussion mailing list
[hidden email]
http://www.egnite.de/mailman/listinfo.cgi/en-nut-discussion
Reply | Threaded
Open this post in threaded view
|

Re: NutSleep implementation optimization

Ole Reinhardt
Hi Harald,

I'll call you later on to discuss the new timer API, but for now, let's
discuss this issue:

> 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.

I don't need them from within interrupt context. What I need is a
guaranteed latency from about 1ms from timer-event to timer callback.
Since we use cooperative multitasking timer callbacks from within
interrupt context seems to be the only solution for me.

> 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.

In which case a memory overflow will be caused in real world? This can
only happen if we use realy a lot of timers, right? But you'r right,
this should be avoided if possible.

> 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.

I would realy suggest to remove the timer callback in the old fashion
way (from within interrupt context) completely, but don't remove timer
callbacks that will be processed outside interrupt context at all.

Let's create a new API with the same capabilities. Perhaps let's even
create an api that supports max. 1 callback per timer. We could provide
an extra function to process a callback list.

In this case most applications would never have the need to register a
callback running from within the interrupt context. And the small part
of applications that need such a feature (e.g. my own) needs to be avare
of the possibility to block the whole system.

One other Idea is not to block all interrupts, but the timer interrupt.
This would not completely block the whole system.

Btw: Why doe we alwyys block all interrupts in a critical section and
not only the ones that would influence our code? In case of an UART we
could only block the UART interrupt and still be able to process network
traffic...

Have I missed something?

Bye,

Ole

--
kernel concepts    Tel: +49-271-771091-14
Dreisbachstr. 24   Fax: +49-271-771091-19
D-57250 Netphen    E+ : +49-177-7420433
--

_______________________________________________
En-Nut-Discussion mailing list
[hidden email]
http://www.egnite.de/mailman/listinfo.cgi/en-nut-discussion
Reply | Threaded
Open this post in threaded view
|

Re: NutSleep implementation optimization

Harald Kipp
Hi Ole,

At 08:26 14.06.2005 +0200, you wrote:

>I don't need them from within interrupt context. What I need is a
>guaranteed latency from about 1ms from timer-event to timer callback.
>Since we use cooperative multitasking timer callbacks from within
>interrupt context seems to be the only solution for me.

Exactly.



>>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.
>
>In which case a memory overflow will be caused in real world? This can
>only happen if we use realy a lot of timers, right? But you'r right, this
>should be avoided if possible.

Agreed. The current implementation may not cause any problem
with 99.99% of applications. But it is ugly.



>I would realy suggest to remove the timer callback in the old fashion way
>(from within interrupt context) completely, but don't remove timer
>callbacks that will be processed outside interrupt context at all.

Why? What's the advantage if it's removed?




>Let's create a new API with the same capabilities. Perhaps let's even
>create an api that supports max. 1 callback per timer. We could provide an
>extra function to process a callback list.

Again, where's the advantage?




>In this case most applications would never have the need to register a
>callback running from within the interrupt context. And the small part of
>applications that need such a feature (e.g. my own) needs to be avare of
>the possibility to block the whole system.

Most applications didn't use timer callbacks with the old implementation
either. What I did was to remove _kernel_ timer callbacks from interrupt
context. But we can still add support for application callback within
interrupt context.




>One other Idea is not to block all interrupts, but the timer interrupt.
>This would not completely block the whole system.

If running in interrupt context, all interrupts are blocked unless
we allow nested interrupts. This is supported, but do you really
want to make this mandantory? If yes, we will never be able to
calculate stack usage.



>Btw: Why doe we alwyys block all interrupts in a critical section and not
>only the ones that would influence our code? In case of an UART we could
>only block the UART interrupt and still be able to process network traffic...
>
>Have I missed something?

Either you or me. :-) You are right for drivers. Some of them
already make use of this. And I used this in the new timer code
as well. But the kernel is unable to tell who will call
NutEventPostFromIrq(). Thus, it has to block _all_ interrupts in
NutEventWait().

Harald

_______________________________________________
En-Nut-Discussion mailing list
[hidden email]
http://www.egnite.de/mailman/listinfo.cgi/en-nut-discussion
Reply | Threaded
Open this post in threaded view
|

Re: NutSleep implementation optimization

Ole Reinhardt
Hi Harald,

> >I would realy suggest to remove the timer callback in the old fashion way
> >(from within interrupt context) completely, but don't remove timer
> >callbacks that will be processed outside interrupt context at all.
>
> Why? What's the advantage if it's removed?

At the moment, the timer interrupt is called nearly every ms, right? If
we remove the possibility of callbacks from the interrupt at all, we
will save some piece of code that will be processed every ms. Even if
it's only a lookup if there is a callback registered, this makes some
additional calls every ms. If we move this feature to a second timer, we
make this configurable, as we don't even have to initialize this second
timer if we don't need it at all. Timer0 is always running.

My only goal is to have the shortest possible interrupt handler.

> Most applications didn't use timer callbacks with the old implementation
> either. What I did was to remove _kernel_ timer callbacks from interrupt
> context. But we can still add support for application callback within
> interrupt context.

Ok, one could also make the compilation switchabel with a preprocessor
macro.

But perhaps I have not fully understood your timer concept...

> >One other Idea is not to block all interrupts, but the timer interrupt.
> >This would not completely block the whole system.
>
> If running in interrupt context, all interrupts are blocked unless
> we allow nested interrupts. This is supported, but do you really
> want to make this mandantory? If yes, we will never be able to
> calculate stack usage.

Right, I forgot about this issue... sorry. Nested interrupts are'nt that
fine, but for speeding up NutOS I would hold it in mind. Do we need to
calculate stack usage? What's about the interrupt stack? These parts of
NutOS I'm not that familiar with. Perhaps you could also explain me
these details on phone later?

> >Btw: Why doe we alwyys block all interrupts in a critical section and not
> >only the ones that would influence our code? In case of an UART we could
> >only block the UART interrupt and still be able to process network traffic...
> >
> >Have I missed something?
>
> Either you or me. :-) You are right for drivers. Some of them
> already make use of this. And I used this in the new timer code
> as well. But the kernel is unable to tell who will call
> NutEventPostFromIrq(). Thus, it has to block _all_ interrupts in
> NutEventWait().

Shure. NutEventWait has to do so. My focus was laying on the drivers.

Bye bye,

Ole

--
kernel concepts    Tel: +49-271-771091-14
Dreisbachstr. 24   Fax: +49-271-771091-19
D-57250 Netphen    E+ : +49-177-7420433
--


_______________________________________________
En-Nut-Discussion mailing list
[hidden email]
http://www.egnite.de/mailman/listinfo.cgi/en-nut-discussion
Reply | Threaded
Open this post in threaded view
|

Re: NutSleep implementation optimization

Matthias Ringwald
In reply to this post by Ole Reinhardt
    Hi Harald, Ole,

>> 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.

It looks like we cuold try to focus on different timer categories.
>
> I don't need them from within interrupt context. What I need is a  
> guaranteed latency from about 1ms from timer-event to timer  
> callback. Since we use cooperative multitasking timer callbacks  
> from within interrupt context seems to be the only solution for me.

If someone has _real_ timing constraints and is using a cooperative  
OS, I guess it has to be handled from interrupt context.
BUT.. that should be used in rare occasions and people should now  
what they do. Ole could use another timer, or we could provide
a single hook in the std timer IRQ handler.

Then I can imaging single or repetitive task, that might take a while  
to process, but don't have stringent timing constraints.
For such apps, a user-space timer callback comes in handy. (although,  
basically all such activities could be performed by
a single thread which does timer processing. but I feel its better to  
have this support in the kernel and well tested).

And yes, there are NutSleep and event timeouts. they can be easiliy  
handled (as they are right now) using the user-space
callbacks or could directly handled in the kernel. As I think that  
the user-space callbacks are handy, using them for
NutSleep and Event timeout makes life easier.



>> 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.
>>
>
I don't understand that. You're saying that an app is creating more  
timers that there is available heap before they are relased in the
next thread switch. ? That's bad luck?

On the other hand, yes, I wouldn't expect a context switch to happen  
when starting I timer. But if yes, it could be written in the docs.. :)
If you say reusing elapsed timers when creating new ones, your're  
talking about processing the elapese timers which might lead to a
woken thread with higher priority? if so, then just process timers  
but don't call NuthTreadSwitch/Yield.


>>
>
> I would realy suggest to remove the timer callback in the old  
> fashion way (from within interrupt context) completely, but don't  
> remove timer callbacks that will be processed outside interrupt  
> context at all.
Yepp.

> ..
> In this case most applications would never have the need to  
> register a callback running from within the interrupt context. And  
> the small part of applications that need such a feature (e.g. my  
> own) needs to be avare of the possibility to block the whole system.
>
> One other Idea is not to block all interrupts, but the timer  
> interrupt. This would not completely block the whole system.
Its the other interrupts, you don't care for timer interrupt if you  
are running there.. :)
The blocking comes from IRQ per default beeing disabled during  
processing IRQs which would otherwise get more complicated
and is called nested IRQs.

>
> Btw: Why doe we alwyys block all interrupts in a critical section  
> and not only the ones that would influence our code?
If we know exactly which one, it makes sense. Harald just started  
blocking timer interrupts if changing timer queues.
But this heavily depends on what is done from IRQ context. As we're  
moving stuff out of the IRQ handler, there's
a reduced need for critical sections. (see my mail on NutSleep).


> In case of an UART we could only block the UART interrupt and still  
> be able to process network traffic...
The problem we have on BTnut is more that kernel activity (thread  
reordering, timer irq.. )
blocks all IRQ including the UART altough the UART RX is not doing  
anything evil.


Matthias
_______________________________________________
En-Nut-Discussion mailing list
[hidden email]
http://www.egnite.de/mailman/listinfo.cgi/en-nut-discussion