kernel/sched.c questions

kernel/sched.c questions

[Sardañons, Eliel]

Hello, I would like to know why you put this two functions:
void scheduling_functions_start_here(void) { }
...
void scheduling_functions_end_here(void) { }

why you put 'case TASK_RUNNING'

switch (prev->state) {
                case TASK_INTERRUPTIBLE:
                        if (signal_pending(prev)) {
                                prev->state = TASK_RUNNING;
                                break;
                        }
                default:
                        del_from_runqueue(prev);
                case TASK_RUNNING:
}

and the last one:

in the function schedule() you always use this syntax:

-----
if (a_condition)
    goto bebe;
bebe_back


bebe:
    do_bebe();
    goto bebe_back;
------
why not just doing:
   
   if (a_condition)
         do_bebe();


I know that goto's are better but finaly you are jumping to a function and
then calling the function. I think you can improve performance doing this.

Re: kernel/sched.c questions

[Tim Walberg]

[-- Attachment #1: Type: text/plain, Size: 2426 bytes --]

On 04/04/2001 16:52 -0300, Sardañons, Eliel wrote:
>>	Hello, I would like to know why you put this two functions:
>>	void scheduling_functions_start_here(void) { }
>>	...
>>	void scheduling_functions_end_here(void) { }
>>	

That one I have no idea about - maybe some perverse sort
of comment? Or maybe something somewhere needs to know the
address range that some functions lie within, and these functions
would delimit that range. Of course, that presumes that the
compiler in use doesn't reorder functions in the object code
that emits, but I think that's a fairly safe assumption for
now...

>>	why you put 'case TASK_RUNNING'
>>	
>>	switch (prev->state) {
>>	                case TASK_INTERRUPTIBLE:
>>	                        if (signal_pending(prev)) {
>>	                                prev->state = TASK_RUNNING;
>>	                                break;
>>	                        }
>>	                default:
>>	                        del_from_runqueue(prev);
>>	                case TASK_RUNNING:
>>	}
>>	


This just indicates that there is nothing to be done for the
TASK_RUNNING case - if it were left out, the default case would
be taken. Of course, a 'case TASK_RUNNING: break;' placed earlier
in the switch construct would be semantically the same, but there
may be reasons related to code optimization that this was done the
way it was.

>>	and the last one:
>>	
>>	in the function schedule() you always use this syntax:
>>	
>>	-----
>>	if (a_condition)
>>	    goto bebe;
>>	bebe_back
>>	
>>	
>>	bebe:
>>	    do_bebe();
>>	    goto bebe_back;
>>	------
>>	why not just doing:
>>	   
>>	   if (a_condition)
>>	         do_bebe();
>>	
>>	
>>	I know that goto's are better but finaly you are jumping to a function and
>>	then calling the function. I think you can improve performance doing this.


This looks like a hand-optimization to avoid a branch in the most common
case. Chances are a_condition is supposed to be pretty rare, and the code
you suggest would usually include a branch for the usual code path, then.


			tw


-- 
+--------------------------+------------------------------+
| Tim Walberg              | tewalberg@mediaone.net       |
| 828 Marshall Ct.         | www.concentric.net/~twalberg |
| Palatine, IL 60074       |                              |
+--------------------------+------------------------------+

[-- Attachment #2: Type: application/pgp-signature, Size: 175 bytes --]

Re: kernel/sched.c questions

[Andi Kleen]

Tim Walberg <tewalberg@mediaone.net> writes:

> On 04/04/2001 16:52 -0300, Sardañons, Eliel wrote:
> >>	Hello, I would like to know why you put this two functions:
> >>	void scheduling_functions_start_here(void) { }
> >>	...
> >>	void scheduling_functions_end_here(void) { }
> >>	
> 
> That one I have no idea about - maybe some perverse sort
> of comment? Or maybe something somewhere needs to know the
> address range that some functions lie within, and these functions
> would delimit that range. Of course, that presumes that the
> compiler in use doesn't reorder functions in the object code
> that emits, but I think that's a fairly safe assumption for
> now...

This is needed for a very bad hack to get the EIP information in ps -lax:
most programs would be shown as hanging in schedule(), which would not be 
very useful to show the user. To avoid this sched.c is always compiled with 
frame pointers and if the EIP is inside these two functions the proc code 
goes back one level in the stack frame.


-Andi

Re: kernel/sched.c questions

[Bjorn Wesen]

On 4 Apr 2001, Andi Kleen wrote:
> > >>	Hello, I would like to know why you put this two functions:
> > >>	void scheduling_functions_start_here(void) { }
> > >>	...
> > >>	void scheduling_functions_end_here(void) { }

> This is needed for a very bad hack to get the EIP information in ps -lax:
> most programs would be shown as hanging in schedule(), which would not be 
> very useful to show the user. To avoid this sched.c is always compiled with 
> frame pointers and if the EIP is inside these two functions the proc code 
> goes back one level in the stack frame.

That sure is a very bad hack :) (For the original poster: search for
get_wchan in the various ports)

There is no comment anywhere near it that says what it is MEANT to do. You
can guess from the code and the usage that it has to do with stack-frames
and special-casing the scheduler functions..  Thanks for the 
clarification.. now I can go and fix it in arch/cris :) (I had never seen
the WCHAN field in ps before actually)

Just as a reference (everyone should get their daily dose of headache)
here is the i386 version:

unsigned long get_wchan(struct task_struct *p)
{
        unsigned long ebp, esp, eip;
        unsigned long stack_page;
        int count = 0;
        if (!p || p == current || p->state == TASK_RUNNING)
                return 0;
        stack_page = (unsigned long)p;
        esp = p->thread.esp;
        if (!stack_page || esp < stack_page || esp > 8188+stack_page)
                return 0;
        /* include/asm-i386/system.h:switch_to() pushes ebp last. */
        ebp = *(unsigned long *) esp;
        do {
                if (ebp < stack_page || ebp > 8184+stack_page)
                        return 0;
                eip = *(unsigned long *) (ebp+4);
                if (eip < first_sched || eip >= last_sched)
                        return eip;
                ebp = *(unsigned long *) ebp;
        } while (count++ < 16);
        return 0;
}

-BW

Re: kernel/sched.c questions

[Richard B. Johnson]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=US-ASCII, Size: 811 bytes --]

On Wed, 4 Apr 2001, Tim Walberg wrote:

> On 04/04/2001 16:52 -0300, Sardañons, Eliel wrote:
> >>	Hello, I would like to know why you put this two functions:
> >>	void scheduling_functions_start_here(void) { }
> >>	...
> >>	void scheduling_functions_end_here(void) { }
> >>	
> 

This is so 'ps' knows if somebody is sleeping in the scheduler,
which is most often the case unless you have 2 or more CPUs.
When these addresses are found, the observed stack is unwound
to find the return address, hense where the sleeping task
was really sleeping.


Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).

"Memory is like gasoline. You use it up when you are running. Of
course you get it all back when you reboot..."; Actual explanation
obtained from the Micro$oft help desk.

Re: kernel/sched.c questions

[Stuart MacDonald]

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

I had similar questions recently when I was doing some
hacking; these are my guesses:

From: <Sardañons>; "Eliel" <Eliel.Sardanons@philips.edu.ar>
> Hello, I would like to know why you put this two functions:
> void scheduling_functions_start_here(void) { }
> ...
> void scheduling_functions_end_here(void) { }

Just as markers for easy location in System.map.
The compiler should optimise those away.

> why you put 'case TASK_RUNNING'
>
> switch (prev->state) {
>                 case TASK_INTERRUPTIBLE:
>                         if (signal_pending(prev)) {
>                                 prev->state = TASK_RUNNING;
>                                 break;
>                         }
>                 default:
>                         del_from_runqueue(prev);
>                 case TASK_RUNNING:
> }

Prevent compiler warnings about unhandled conditions?
Not sure about that one.

> in the function schedule() you always use this syntax:
>
> -----
> if (a_condition)
>     goto bebe;
> bebe_back
>
>
> bebe:
>     do_bebe();
>     goto bebe_back;
> ------
> why not just doing:
>
>    if (a_condition)
>          do_bebe();

Probably because the compiler puts out

setup function parameter one
setup function parameter two
setup function parameter three
check condition
call function
setup function parameter one
setup function parameter two
setup function parameter three
check condition
call function

for your case and the above convolutions
puts out

check condition
jump to call if needed
check condition
jump to call if needed

instead.

Or even if the compiler puts out

check condition
If condition
  setup function parameter one
  setup function parameter two
  setup function parameter three
  call function
check condition
if condition
  setup function parameter one
  setup function parameter two
  setup function parameter three
  call function

I'm betting the smaller code above is better
for cache hits, right?

But these are my guesses. Anyone want to
clarify?

..Stu

Re: kernel/sched.c questions

[Steven Walter]

On Wed, Apr 04, 2001 at 04:52:32PM -0300, Sarda?ons, Eliel wrote:
> switch (prev->state) {
>                 case TASK_INTERRUPTIBLE:
>                         if (signal_pending(prev)) {
>                                 prev->state = TASK_RUNNING;
>                                 break;
>                         }
>                 default:
>                         del_from_runqueue(prev);
>                 case TASK_RUNNING:
> }

I'm not sure about the other two, but this one is pretty straight
forward:  its listed explicitly because we don't want tasks with 
p->state TASK_RUNNING to fall into the default case, that is, getting
deleted from the runqueue.  This would be bad.

-- 
-Steven
Freedom is the freedom to say that two plus two equals four.