Re: [PATCH] Replaces two GOTO statements with one IF_ELSE statement in /fs/open.c

[Mitchell Blank Jr]

Richard B. Johnson wrote:
> Did you check and benchmark the resulting code? The goto statements
> are never because the kernel developers didn't know how to use
> other constructs, you know.
> 
> The goto statements are so that the normal path continues
> straight through the code while the exceptions take the jumps.

Yes, but I think in this case it's OK.  IS_ERR() already includes an
"unlikely()" marking so gcc *should* be able to do the right thing.
The goto is probably just a remnant from before unlikely() existed.
There are a lot of them - cleaning them up is a good thing.

That said, it'd probably be good to look at the generated assembly in each
case to make sure it still looks good.  That'd be a lot more useful than
trying to benchmark a tiny micro-optimizaiton in open()  It's generally
nearly impossible to benchmark these sorts of things anyway since when
you're running a simple benchmark the whole function will be in Icache and
the branch predictor history so it'll run the same speed either way.

The advantage to keeping the fast-path straight through is that you consume
the minimum number of lines in the Icache and the branch will get predicted
correctly even without history (since the first-level branch prediction
heuristic is always "branches backward are taken, branches forward are not")

-Mitch
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/