Re: Syscall kill(2) called for a zombie process should return 0

[David Laight <david%l8s.co.uk@localhost>]

On Wed, Jul 18, 2012 at 12:37:58PM +0100, Roy Marples wrote:
> 
> +                     if (p != NULL && P_ZOMBIE(p)) {
> +                             mutex_exit(proc_lock);
> +                             return 0;
> +                     }
>                       mutex_exit(proc_lock);
>                       return ESRCH;
> 
> This is a general question, not necessarily specific to the patch.
> Which is more costly? Two function calls as above, or storing the return
> value in a variable to return with just one function call to mutex_exit?

My tuppence:

The compiler will normally have a callee saved register to save the return
value in, so that won't need any stack accesses - just register moves.

The returns might be compiled as jumps to other code that sets the
same return value and then to a common prologue, or might contain
the full function prologue. If jumps are expensive, or the target
code isn't in the cache, those jumps will have significant cost.

My guess is that the compiler will generate 2 calls to mutex_exit()
for the above - the optimiser won't have the pattern to convert it to:
        rval = ESRCH;
        if (p != NULL && P_ZOMBIE(p))
                rval = 0;
        mutex_exit();
        return rval;

Even if you add __builtin_expect() to the above, gcc will tend to make the
rval = 0 path the fallthrough one. To force the 'p == NULL' case to be
the fallthrough one, you need to have a non-empty else part on the if
        asm volatile ("# comment" STR(__LINE__) "\n");
will probably do.
gcc seemed to size the asm code by the number of lines, adding multiple
\n made a difference!

I was recently doing some clock-counting optimisations.
For code like:
        for (;;) {
                ...
                if (foo)
                        bar = 1;
                baz = 2;
                continue;
I needed to ensure each branch of the 'if' only had one jump.
It wasn't enough to do:
                if (foo) {
                        bar = 1;
                        baz = 2;
                        continue;
                }
                baz = 2;
                continue;
because the compiler will merge the code.
An asm comment before the continue did the trick.

I could have written the whole code in assembler, but the compiler
does a better job of not reusing registers that contain important
values, and I make less silly mistakes in C.

        David

-- 
David Laight: david%l8s.co.uk@localhost