[ECOS] ecos 1.3.1

[ECOS] ecos 1.3.1

[Rafael Rodríguez Velilla]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 724 bytes --]

  In the implementation of MLQUEUE scheduler I've found that the method
rem_thread doesn't check if the thread that is being removed is the
"current_thread". Shouldn't this check be done in order to raise
"need_reschedule" when that happens?
  In the BITMAP scheme it is done.

Another little thing, in the method add_thread I find the following
code:
if (thread->queue != NULL)
{
   thread->queue->remove(thread);
   thread->queue=NULL;
}

 Isn't it true that when remove is invoqued, the member queue of the
thread used as a parameter finishes pointing to NULL? So thread->queue
is innecessary.


TIA

--
Rafael Rodríguez Velilla        rrv@tid.es
Telefónica I+D          http://www.tid.es
Telf: +34 - 91 337 4270

Re: [ECOS] ecos 1.3.1

[Jonathan Larmour]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1195 bytes --]

Rafael Rodríguez Velilla wrote:
> 
>   In the implementation of MLQUEUE scheduler I've found that the method
> rem_thread doesn't check if the thread that is being removed is the
> "current_thread". Shouldn't this check be done in order to raise
> "need_reschedule" when that happens?
>   In the BITMAP scheme it is done.

I believe that the kernel expects the caller of rem_thread to always
manipulate the thread state to be something other than RUNNING. And given
that rem_thread should always be called with the scheduler locked, this
should force a reschedule. The bitmap scheduler is probably just overly
zealous. Nick, comments?

> Another little thing, in the method add_thread I find the following
> code:
> if (thread->queue != NULL)
> {
>    thread->queue->remove(thread);
>    thread->queue=NULL;
> }
> 
>  Isn't it true that when remove is invoqued, the member queue of the
> thread used as a parameter finishes pointing to NULL? So thread->queue
> is innecessary.

Looks like it. Nick, any comments before I change it?

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine

Re: [ECOS] ecos 1.3.1

[Nick Garnett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1774 bytes --]

Jonathan Larmour <jlarmour@redhat.com> writes:

> Rafael Rodríguez Velilla wrote:
> > 
> >   In the implementation of MLQUEUE scheduler I've found that the method
> > rem_thread doesn't check if the thread that is being removed is the
> > "current_thread". Shouldn't this check be done in order to raise
> > "need_reschedule" when that happens?
> >   In the BITMAP scheme it is done.
> 
> I believe that the kernel expects the caller of rem_thread to always
> manipulate the thread state to be something other than RUNNING. And given
> that rem_thread should always be called with the scheduler locked, this
> should force a reschedule. The bitmap scheduler is probably just overly
> zealous. Nick, comments?

This is correct. The original hope was that we could always use
need_reschedule to drive the decision to reschedule. However, that
proved to be impossible, and a further test was added in
unlock_inner() for whether the current thread is still runnable. The
test in the mlqueue scheduler that originally tested this has been
removed, but obviously the one in the bitmap scheduler got
overlooked. It theory it could probably be removed from there too.

> 
> > Another little thing, in the method add_thread I find the following
> > code:
> > if (thread->queue != NULL)
> > {
> >    thread->queue->remove(thread);
> >    thread->queue=NULL;
> > }
> > 
> >  Isn't it true that when remove is invoqued, the member queue of the
> > thread used as a parameter finishes pointing to NULL? So thread->queue
> > is innecessary.
> 
> Looks like it. Nick, any comments before I change it?
>

Double check that this is still true in the CVS sources rather than
1.3.1. A lot has changed in this code since then.

-- 
Nick Garnett, eCos Kernel Architect
Red Hat, Cambridge, UK