|
From: Kristian V. D. V. <va...@li...> - 2005-01-21 08:00:17
|
On Thursday 20 January 2005 22:56, Daniel Gryniewicz wrote:
> Here's the first in a series of cleanup patches for the scheduler. This
> is all (hopefully) preperatory to porting Con Kolivas' staircase
> scheduler from the linux -ck kernel. This is the scheduler I use on all
> my linux boxes, and it's absolutely great for desktops.
This sounds excelent. I'm happy to see some real interest in the kernel these
days.
> Attached is the patch and the list header, which is assumed to be in
> inc/list.h
Could we move it to <atheos/list.h>? I can think of a few other places in the
kernel and the drivers where a generic linked list implementation would be
useful.
> I've tested this fairly thouroughly, and it works.
I have a few comments on the patch. Hope you don't mind!
1. In add_thread_to_ready() you have
for ( psTmp = LIST_FIRST( &g_sSysBase.ex_sFirstReady ); NULL != psTmp; psTmp
= LIST_NEXT( psTmp, tr_psNext ) )
{
...
if ( LIST_NEXT( psTmp, tr_psNext ) == NULL )
break;
}
It looks to me as though you call LIST_NEXT twice within the loop; once
within the for() and then again at the bottom of the loop. Should this not
be called once, perhaps at the bottom of the loop E.g.
for ( psTmp = LIST_FIRST( &g_sSysBase.ex_sFirstReady ); NULL != psTmp; )
{
...
psTmp = LIST_NEXT( psTmp, tr_psNext );
if ( psTmp == NULL )
break;
}
2. Possibly rename g_SysBase.ex_sFirstReady; to E.g. ex_ReadyListHead or
similiar? Don't worry about this one if it's going to get replaced by the
new scheduler changes anyway.
> The second patch
> will follow after I've finished testing it. After that, my third patch
> makes all WaitQueue_s objects use the list primatives as well. After
> that, I'll start making actual changes to the scheduler.
Those sound good to me.
--
Vanders
http://www.syllable.org
http://www.liqwyd.com
|