Re: [Syllable-kernel] Scheduler cleanup

[Kristian V. D. V. <va...@li...>]

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