Re: [2.4.28-rc1] process stuck in release_task() call

[Marcelo Tosatti <marcelo.tosatti@cyclades.com>]

On Thu, Nov 11, 2004 at 09:33:12AM +0100, Willy Tarreau wrote:
> Hi Marcelo,
> 
> > > >>EIP; c012073d <release_task+1fd/230>   <=====
> (...)
> > > c0120540 <release_task>:
> > > c0120540:       55                      push   %ebp
> > > ....
> > > c0120736:       89 d8                   mov    %ebx,%eax
> > > c0120738:       e8 73 dd 01 00          call   c013e4b0 <free_pages> <= here
> > 
> > is this release_task+1fd?  Can you send me the full disassemble of release_task?
> 
> Yes it is because the next instruction after call will be at c0120738+5 =
> c012073d = release_task+1fd. (the return address on the stack is the
> address of the next instruction after the call).

OK.

> > It can't be blocked here, its a "call" instruction. 
> 
> Seems rather strange indeed ! Perhaps this is not the disassembled function
> of the *running* kernel ? it would be good to disassemble vmlinux and ensure
> that it is exactly the one currently running. I too have already lost lots
> of time searching a wrong bug because I disassembled the wrong kernel, so
> I'm certain it can happen even when we're very careful :-(
> 
> > free_pages can't block either. Odd.  
> 
> Marcelo, I have two questions for my own understanding :
>   - free_pages does spin_lock(&zone->lock) around the while() loop.
>     Considering that someone else could hold the lock (bug, etc...), it
>     could block here. But my feeling is that if such a lock were kept held,
>     the system would be totally frozen because everything which would want
>     to free memory would get stuck (even a process exit). Am I right ?

Right, the system will be totally frozen spinning on the lock.

>   - would it enhance performance a bit to put a bunch of 'unlikely()' in all
>     the ifs which end in BUG(), especially inside the loop ?

Yes, it should generate better code. 

Try it and see how the generated code differs from the original without unlikely.

I'm not aware of the internals of unlikely however, so I can't 
explain how it works in details... the GCC documentation 
should do it.  :)